From afbbe72b6d1a5d61f334647946376f9fb8885c7c Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Mon, 28 Jun 2021 10:42:07 -0400 Subject: [PATCH 1/8] Refactor unnecessary list of tracks in wtrackmenu -- it's only ever one track. --- src/widget/wtrackmenu.cpp | 63 +++++++++++++++------------------------ src/widget/wtrackmenu.h | 4 +-- 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/widget/wtrackmenu.cpp b/src/widget/wtrackmenu.cpp index 9fb3fee4aa0b..4fccccbc7055 100644 --- a/src/widget/wtrackmenu.cpp +++ b/src/widget/wtrackmenu.cpp @@ -80,7 +80,7 @@ int WTrackMenu::getTrackCount() const { if (m_pTrackModel) { return m_trackIndexList.size(); } else { - return m_trackPointerList.size(); + return m_pTrack ? 1 : 0; } } @@ -508,10 +508,8 @@ bool WTrackMenu::isAnyTrackBpmLocked() const { } } } else { - for (const auto& pTrack : m_trackPointerList) { - if (pTrack->isBpmLocked()) { - return true; - } + if (m_pTrack && m_pTrack->isBpmLocked()) { + return true; } } return false; @@ -536,13 +534,10 @@ std::optional> WTrackMenu::getCommonTrackColor() } } } else { - commonColor = m_trackPointerList.first()->getColor(); - for (const auto& pTrack : m_trackPointerList) { - if (commonColor != pTrack->getColor()) { - // Multiple, different colors - return std::nullopt; - } + if (!m_pTrack) { + return std::nullopt; } + commonColor = m_pTrack->getColor(); } return make_optional(commonColor); } @@ -605,7 +600,7 @@ CoverInfo WTrackMenu::getCoverInfoOfLastTrack() const { .toString(); return coverInfo; } else { - return m_trackPointerList.last()->getCoverInfoWithLocation(); + return m_pTrack->getCoverInfoWithLocation(); } } @@ -755,7 +750,7 @@ void WTrackMenu::loadTrack( if (!pTrack) { return; } - m_trackPointerList = TrackPointerList{pTrack}; + m_pTrack = pTrack; updateMenus(); } @@ -788,9 +783,8 @@ TrackIdList WTrackMenu::getTrackIds() const { trackIds.push_back(trackId); } } else { - trackIds.reserve(m_trackPointerList.size()); - for (const auto& pTrack : m_trackPointerList) { - const auto trackId = pTrack->getId(); + if (m_pTrack) { + const auto trackId = m_pTrack->getId(); DEBUG_ASSERT(trackId.isValid()); trackIds.push_back(trackId); } @@ -812,15 +806,11 @@ QList WTrackMenu::getTrackRefs() const { } trackRefs.push_back(std::move(trackRef)); } - } else { - trackRefs.reserve(m_trackPointerList.size()); - for (const auto& pTrack : m_trackPointerList) { - DEBUG_ASSERT(pTrack); - auto trackRef = TrackRef::fromFileInfo( - pTrack->getFileInfo(), - pTrack->getId()); - trackRefs.push_back(std::move(trackRef)); - } + } else if (m_pTrack) { + auto trackRef = TrackRef::fromFileInfo( + m_pTrack->getFileInfo(), + m_pTrack->getId()); + trackRefs.push_back(std::move(trackRef)); } return trackRefs; } @@ -835,13 +825,10 @@ TrackPointer WTrackMenu::getFirstTrackPointer() const { // Skip unavailable tracks } return TrackPointer(); - } else { - if (m_trackPointerList.isEmpty()) { - return TrackPointer(); - } - DEBUG_ASSERT(m_trackPointerList.first()); - return m_trackPointerList.first(); + } else if (m_pTrack) { + return m_pTrack; } + return TrackPointer(); } std::unique_ptr WTrackMenu::newTrackPointerIterator() const { @@ -854,13 +841,11 @@ std::unique_ptr WTrackMenu::newTrackPointerIterator return std::make_unique( m_pTrackModel, m_trackIndexList); - } else { - if (m_trackPointerList.isEmpty()) { - return nullptr; - } + } else if (m_pTrack) { return std::make_unique( - m_trackPointerList); + TrackPointerList{m_pTrack}); } + return nullptr; } int WTrackMenu::applyTrackPointerOperation( @@ -1597,7 +1582,7 @@ void WTrackMenu::slotShowDlgTrackInfo() { if (m_pTrackModel) { m_pDlgTrackInfo->loadTrack(m_trackIndexList.at(0)); } else { - m_pDlgTrackInfo->loadTrack(m_trackPointerList.at(0)); + m_pDlgTrackInfo->loadTrack(m_pTrack); } m_pDlgTrackInfo->show(); } @@ -1621,7 +1606,7 @@ void WTrackMenu::slotShowDlgTagFetcher() { if (m_pTrackModel) { m_pDlgTagFetcher->loadTrack(m_trackIndexList.at(0)); } else { - m_pDlgTagFetcher->loadTrack(m_trackPointerList.at(0)); + m_pDlgTagFetcher->loadTrack(m_pTrack); } m_pDlgTagFetcher->show(); } @@ -1737,7 +1722,7 @@ void WTrackMenu::slotPurge() { } void WTrackMenu::clearTrackSelection() { - m_trackPointerList.clear(); + m_pTrack = nullptr; m_trackIndexList.clear(); } diff --git a/src/widget/wtrackmenu.h b/src/widget/wtrackmenu.h index ced9c065546b..54c8fb465b67 100644 --- a/src/widget/wtrackmenu.h +++ b/src/widget/wtrackmenu.h @@ -186,8 +186,8 @@ class WTrackMenu : public QMenu { TrackModel* const m_pTrackModel; QModelIndexList m_trackIndexList; - // Source of track list when TrackModel is not set. - TrackPointerList m_trackPointerList; + /// Track being referenced when TrackModel is not set. + TrackPointer m_pTrack; const ControlProxy* m_pNumSamplers{}; const ControlProxy* m_pNumDecks{}; From f6e6b68bcd6216be39c27c7cdf0563d01df5b287 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Mon, 28 Jun 2021 10:42:43 -0400 Subject: [PATCH 2/8] Adjust ReplayGain: Allow user to update the replaygain value based on a deck pregain value. Currently accessible by right clicking loaded decks, including preview deck --- src/mixer/basetrackplayer.cpp | 18 +++++++++++++++--- src/mixer/basetrackplayer.h | 3 ++- src/track/replaygain.h | 9 +++++++-- src/track/track.cpp | 24 +++++++++++++++++++++--- src/track/track.h | 5 ++++- src/widget/wtrackmenu.cpp | 32 +++++++++++++++++++++++++++++++- src/widget/wtrackmenu.h | 10 +++++++++- src/widget/wtrackproperty.cpp | 7 ++++--- src/widget/wtracktext.cpp | 7 ++++--- src/widget/wtrackwidgetgroup.cpp | 5 +++-- 10 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index 63313d16e219..355b29cc58fe 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -597,11 +597,23 @@ void BaseTrackPlayerImpl::slotCloneChannel(EngineChannel* pChannel) { slotLoadTrack(pTrack, false); } -void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain) { +void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain, + mixxx::ReplayGain::ReplayGainUpdateMode mode) { // Do not change replay gain when track is playing because // this may lead to an unexpected volume change - if (m_pPlay->get() == 0.0) { - setReplayGain(replayGain.getRatio()); + if (m_pPlay->get() == 0.0 || mode == mixxx::ReplayGain::UpdateAndAdjustGain) { + const double factor = m_pReplayGain->get() / replayGain.getRatio(); + const double newPregain = m_pPreGain->get() * factor; + // There is a very slight chance that there will be a buffer call in between these sets. + // Therefore, we first adjust the control that is being lowered before the control + // that is being raised. Worst case, the volume goes down briefly before rectifying. + if (newPregain <= replayGain.getRatio()) { + m_pPreGain->set(newPregain); + setReplayGain(replayGain.getRatio()); + } else { + setReplayGain(replayGain.getRatio()); + m_pPreGain->set(newPregain); + } } else { m_replaygainPending = true; } diff --git a/src/mixer/basetrackplayer.h b/src/mixer/basetrackplayer.h index 87bceec0b749..165dcd2c5e1a 100644 --- a/src/mixer/basetrackplayer.h +++ b/src/mixer/basetrackplayer.h @@ -81,7 +81,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { void slotCloneDeck() final; void slotTrackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack); void slotLoadFailed(TrackPointer pTrack, const QString& reason); - void slotSetReplayGain(mixxx::ReplayGain replayGain); + void slotSetReplayGain(mixxx::ReplayGain replayGain, + mixxx::ReplayGain::ReplayGainUpdateMode mode); void slotSetTrackColor(const mixxx::RgbColor::optional_t& color); void slotPlayToggled(double); diff --git a/src/track/replaygain.h b/src/track/replaygain.h index 76c3e51fa52a..a72af495c616 100644 --- a/src/track/replaygain.h +++ b/src/track/replaygain.h @@ -99,7 +99,12 @@ class ReplayGain final { m_peak = normalizePeak(m_peak); } -private: + enum ReplayGainUpdateMode { + UpdateWhenStopped, + UpdateAndAdjustGain + }; + + private: double m_ratio; CSAMPLE m_peak; }; @@ -119,7 +124,7 @@ QDebug operator<<(QDebug dbg, const ReplayGain& arg) { return dbg << "ratio =" << arg.getRatio() << "/" << "peak =" << arg.getPeak(); } -} +} // namespace mixxx Q_DECLARE_TYPEINFO(mixxx::ReplayGain, Q_MOVABLE_TYPE); Q_DECLARE_METATYPE(mixxx::ReplayGain) diff --git a/src/track/track.cpp b/src/track/track.cpp index 2402aaf259a5..23f7b031f7d9 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -207,7 +207,7 @@ void Track::replaceMetadataFromSource( emit keyChanged(); } if (oldReplayGain != newReplayGain) { - emit replayGainUpdated(newReplayGain); + emit replayGainUpdated(newReplayGain, mixxx::ReplayGain::UpdateWhenStopped); } if (colorModified) { emit colorUpdated(newColor); @@ -305,7 +305,7 @@ bool Track::replaceRecord( emit keyChanged(); } if (oldReplayGain != newReplayGain) { - emit replayGainUpdated(newReplayGain); + emit replayGainUpdated(newReplayGain, mixxx::ReplayGain::UpdateWhenStopped); } if (oldColor != newColor) { emit colorUpdated(newColor); @@ -324,7 +324,25 @@ void Track::setReplayGain(const mixxx::ReplayGain& replayGain) { auto locked = lockMutex(&m_qMutex); if (compareAndSet(m_record.refMetadata().refTrackInfo().ptrReplayGain(), replayGain)) { markDirtyAndUnlock(&locked); - emit replayGainUpdated(replayGain); + emit replayGainUpdated(replayGain, mixxx::ReplayGain::UpdateWhenStopped); + } +} + +void Track::adjustReplayGainFromDeckGain(const QString& deckGroup) { + QMutexLocker lock(&m_qMutex); + mixxx::ReplayGain replayGain = m_record.getMetadata().getTrackInfo().getReplayGain(); + auto deckPregainCO = ControlProxy(deckGroup, "pregain"); + if (!deckPregainCO.valid()) { + qDebug() << "pregain CO not valid"; + return; + } + const double gain = deckPregainCO.get(); + qDebug() << "ADJUST REPLAYGAIN: old: " << replayGain.getRatio() + << " adjust " << gain << " so: " << replayGain.getRatio() * gain; + replayGain.setRatio(replayGain.getRatio() * gain); + if (compareAndSet(m_record.refMetadata().refTrackInfo().ptrReplayGain(), replayGain)) { + markDirtyAndUnlock(&lock); + emit replayGainUpdated(replayGain, mixxx::ReplayGain::UpdateAndAdjustGain); } } diff --git a/src/track/track.h b/src/track/track.h index 5dab8dd708f0..1f174eafa30e 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -144,6 +144,8 @@ class Track : public QObject { // Set ReplayGain void setReplayGain(const mixxx::ReplayGain&); + // Adjust ReplayGain by the value from the given deck's pregain knob. + void adjustReplayGainFromDeckGain(const QString& deckGroup); // Returns ReplayGain mixxx::ReplayGain getReplayGain() const; @@ -409,7 +411,8 @@ class Track : public QObject { void waveformSummaryUpdated(); void coverArtUpdated(); void beatsUpdated(); - void replayGainUpdated(mixxx::ReplayGain replayGain); + void replayGainUpdated(mixxx::ReplayGain replayGain, + mixxx::ReplayGain::ReplayGainUpdateMode mode); void colorUpdated(const mixxx::RgbColor::optional_t& color); void cuesUpdated(); void analyzed(); diff --git a/src/widget/wtrackmenu.cpp b/src/widget/wtrackmenu.cpp index 4fccccbc7055..802078db3080 100644 --- a/src/widget/wtrackmenu.cpp +++ b/src/widget/wtrackmenu.cpp @@ -340,6 +340,15 @@ void WTrackMenu::createActions() { &WTrackMenu::slotClearBeats); } + if (featureIsEnabled(Feature::UpdateReplayGain)) { + m_pUpdateReplayGain = + new QAction(tr("Update ReplayGain from Deck Gain"), m_pClearMetadataMenu); + connect(m_pUpdateReplayGain, + &QAction::triggered, + this, + &WTrackMenu::slotUpdateReplaygain); + } + if (featureIsEnabled(Feature::Color)) { ColorPaletteSettings colorPaletteSettings(m_pConfig); m_pColorPickerAction = new WColorPickerAction(WColorPicker::Option::AllowNoColor, @@ -473,6 +482,10 @@ void WTrackMenu::setupActions() { addMenu(m_pClearMetadataMenu); } + if (featureIsEnabled(Feature::UpdateReplayGain)) { + addAction(m_pUpdateReplayGain); + } + addSeparator(); if (featureIsEnabled(Feature::HideUnhidePurge)) { if (m_pTrackModel->hasCapabilities(TrackModel::Capability::Hide)) { @@ -701,6 +714,10 @@ void WTrackMenu::updateMenus() { } } + if (featureIsEnabled(Feature::UpdateReplayGain)) { + m_pUpdateReplayGain->setEnabled(!m_deckGroup.isEmpty()); + } + if (featureIsEnabled(Feature::Color)) { m_pColorPickerAction->setColorPalette( ColorPaletteSettings(m_pConfig).getTrackColorPalette()); @@ -736,7 +753,7 @@ void WTrackMenu::updateMenus() { } void WTrackMenu::loadTrack( - const TrackPointer& pTrack) { + const TrackPointer& pTrack, const QString& deckGroup) { // This asserts that this function is only accessible when a track model is not set, // thus maintaining only the TrackPointerList in state and avoiding storing // duplicate state with TrackIdList and QModelIndexList. @@ -751,6 +768,7 @@ void WTrackMenu::loadTrack( return; } m_pTrack = pTrack; + m_deckGroup = deckGroup; updateMenus(); } @@ -899,6 +917,17 @@ class ImportMetadataFromFileTagsTrackPointerOperation : public mixxx::TrackPoint } // anonymous namespace +void WTrackMenu::slotUpdateReplaygain() { + if (!m_pTrack) { + qDebug() << "track pointer nullptr, returning"; + return; + } + if (m_deckGroup.isEmpty()) { + qDebug() << "deck group not set"; + } + m_pTrack->adjustReplayGainFromDeckGain(m_deckGroup); +} + void WTrackMenu::slotImportMetadataFromFileTags() { const auto progressLabelText = tr("Importing metadata of %n track(s) from file tags", "", getTrackCount()); @@ -1723,6 +1752,7 @@ void WTrackMenu::slotPurge() { void WTrackMenu::clearTrackSelection() { m_pTrack = nullptr; + m_deckGroup.clear(); m_trackIndexList.clear(); } diff --git a/src/widget/wtrackmenu.h b/src/widget/wtrackmenu.h index 54c8fb465b67..5e232d7c36e3 100644 --- a/src/widget/wtrackmenu.h +++ b/src/widget/wtrackmenu.h @@ -47,6 +47,7 @@ class WTrackMenu : public QMenu { FileBrowser = 1 << 10, Properties = 1 << 11, SearchRelated = 1 << 12, + UpdateReplayGain = 1 << 13, TrackModelFeatures = Remove | HideUnhidePurge, All = AutoDJ | LoadTo | Playlist | Crate | Remove | Metadata | Reset | BPM | Color | HideUnhidePurge | FileBrowser | Properties | @@ -69,7 +70,7 @@ class WTrackMenu : public QMenu { const QModelIndexList& trackIndexList); void loadTrack( - const TrackPointer& pTrack); + const TrackPointer& pTrack, const QString& deckGroup); // WARNING: This function hides non-virtual QMenu::popup(). // This has been done on purpose to ensure menu doesn't popup without loaded track(s). @@ -106,6 +107,7 @@ class WTrackMenu : public QMenu { void slotScaleBpm(mixxx::Beats::BpmScale scale); // Info and metadata + void slotUpdateReplaygain(); void slotShowDlgTagFetcher(); void slotImportMetadataFromFileTags(); void slotExportMetadataIntoFileTags(); @@ -188,6 +190,9 @@ class WTrackMenu : public QMenu { /// Track being referenced when TrackModel is not set. TrackPointer m_pTrack; + /// If the user right clicked on a track in a deck, this will record which + /// deck made the request. + QString m_deckGroup; const ControlProxy* m_pNumSamplers{}; const ControlProxy* m_pNumDecks{}; @@ -207,6 +212,9 @@ class WTrackMenu : public QMenu { WCoverArtMenu* m_pCoverMenu{}; parented_ptr m_pSearchRelatedMenu; + // Update Replaygain from Track + QAction* m_pUpdateReplayGain{}; + // Reload Track Metadata Action: QAction* m_pImportMetadataFromFileAct{}; QAction* m_pImportMetadataFromMusicBrainzAct{}; diff --git a/src/widget/wtrackproperty.cpp b/src/widget/wtrackproperty.cpp index 67cea1b497b7..c4705610a387 100644 --- a/src/widget/wtrackproperty.cpp +++ b/src/widget/wtrackproperty.cpp @@ -19,7 +19,8 @@ constexpr WTrackMenu::Features kTrackMenuFeatures = WTrackMenu::Feature::BPM | WTrackMenu::Feature::Color | WTrackMenu::Feature::FileBrowser | - WTrackMenu::Feature::Properties; + WTrackMenu::Feature::Properties | + WTrackMenu::Feature::UpdateReplayGain; } // namespace WTrackProperty::WTrackProperty( @@ -91,7 +92,7 @@ void WTrackProperty::mouseMoveEvent(QMouseEvent* event) { void WTrackProperty::mouseDoubleClickEvent(QMouseEvent* event) { Q_UNUSED(event); if (m_pCurrentTrack) { - m_pTrackMenu->loadTrack(m_pCurrentTrack); + m_pTrackMenu->loadTrack(m_pCurrentTrack, m_group); m_pTrackMenu->slotShowDlgTrackInfo(); } } @@ -107,7 +108,7 @@ void WTrackProperty::dropEvent(QDropEvent* event) { void WTrackProperty::contextMenuEvent(QContextMenuEvent* event) { event->accept(); if (m_pCurrentTrack) { - m_pTrackMenu->loadTrack(m_pCurrentTrack); + m_pTrackMenu->loadTrack(m_pCurrentTrack, m_group); // Create the right-click menu m_pTrackMenu->popup(event->globalPos()); } diff --git a/src/widget/wtracktext.cpp b/src/widget/wtracktext.cpp index 3c1ddcf7a4cf..93db0d62ff9c 100644 --- a/src/widget/wtracktext.cpp +++ b/src/widget/wtracktext.cpp @@ -19,7 +19,8 @@ constexpr WTrackMenu::Features kTrackMenuFeatures = WTrackMenu::Feature::BPM | WTrackMenu::Feature::Color | WTrackMenu::Feature::FileBrowser | - WTrackMenu::Feature::Properties; + WTrackMenu::Feature::Properties | + WTrackMenu::Feature::UpdateReplayGain; } // namespace WTrackText::WTrackText(QWidget* pParent, @@ -82,7 +83,7 @@ void WTrackText::mouseMoveEvent(QMouseEvent *event) { void WTrackText::mouseDoubleClickEvent(QMouseEvent* event) { Q_UNUSED(event); if (m_pCurrentTrack) { - m_pTrackMenu->loadTrack(m_pCurrentTrack); + m_pTrackMenu->loadTrack(m_pCurrentTrack, m_group); m_pTrackMenu->slotShowDlgTrackInfo(); } } @@ -98,7 +99,7 @@ void WTrackText::dropEvent(QDropEvent *event) { void WTrackText::contextMenuEvent(QContextMenuEvent* event) { event->accept(); if (m_pCurrentTrack) { - m_pTrackMenu->loadTrack(m_pCurrentTrack); + m_pTrackMenu->loadTrack(m_pCurrentTrack, m_group); // Create the right-click menu m_pTrackMenu->popup(event->globalPos()); } diff --git a/src/widget/wtrackwidgetgroup.cpp b/src/widget/wtrackwidgetgroup.cpp index df6fc93dadc4..ebdb9adfd3a3 100644 --- a/src/widget/wtrackwidgetgroup.cpp +++ b/src/widget/wtrackwidgetgroup.cpp @@ -23,7 +23,8 @@ constexpr WTrackMenu::Features kTrackMenuFeatures = WTrackMenu::Feature::BPM | WTrackMenu::Feature::Color | WTrackMenu::Feature::FileBrowser | - WTrackMenu::Feature::Properties; + WTrackMenu::Feature::Properties | + WTrackMenu::Feature::UpdateReplayGain; } // anonymous namespace @@ -121,7 +122,7 @@ void WTrackWidgetGroup::dropEvent(QDropEvent* event) { void WTrackWidgetGroup::contextMenuEvent(QContextMenuEvent* event) { event->accept(); if (m_pCurrentTrack) { - m_pTrackMenu->loadTrack(m_pCurrentTrack); + m_pTrackMenu->loadTrack(m_pCurrentTrack, m_group); // Create the right-click menu m_pTrackMenu->popup(event->globalPos()); } From 7cd5dedc1e5877c68b8e03f5ff66e6cb2a74d957 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Mon, 19 Jul 2021 19:34:37 -0400 Subject: [PATCH 3/8] Replaygain: rework implementation to put the right logic in the right places --- src/mixer/basetrackplayer.cpp | 62 ++++++++++++++++++++++++++--------- src/mixer/basetrackplayer.h | 7 ++-- src/track/replaygain.h | 5 --- src/track/track.cpp | 20 ++++------- src/track/track.h | 10 +++--- src/widget/wtrackmenu.cpp | 19 +++++++---- src/widget/wtrackmenu.h | 2 +- 7 files changed, 76 insertions(+), 49 deletions(-) diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index 355b29cc58fe..8b0ad30de5b6 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -192,6 +192,13 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl( m_pRateRatio = make_parented(getGroup(), "rate_ratio", this); m_pPitchAdjust = make_parented(getGroup(), "pitch_adjust", this); + + m_pUpdateReplayGainFromDeckGain = std::make_unique( + ConfigKey(getGroup(), "update_replaygain_from_pregain")); + connect(m_pUpdateReplayGainFromDeckGain.get(), + &ControlObject::valueChanged, + this, + &BaseTrackPlayerImpl::slotUpdateReplaygainFromDeckGain); } BaseTrackPlayerImpl::~BaseTrackPlayerImpl() { @@ -367,6 +374,10 @@ void BaseTrackPlayerImpl::connectLoadedTrack() { &Track::replayGainUpdated, this, &BaseTrackPlayerImpl::slotSetReplayGain); + connect(m_pLoadedTrack.get(), + &Track::updateAndAdjustReplayGain, + this, + &BaseTrackPlayerImpl::slotUpdateAndAdjustReplayGain); connect(m_pLoadedTrack.get(), &Track::colorUpdated, @@ -597,28 +608,32 @@ void BaseTrackPlayerImpl::slotCloneChannel(EngineChannel* pChannel) { slotLoadTrack(pTrack, false); } -void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain, - mixxx::ReplayGain::ReplayGainUpdateMode mode) { +void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain) { // Do not change replay gain when track is playing because - // this may lead to an unexpected volume change - if (m_pPlay->get() == 0.0 || mode == mixxx::ReplayGain::UpdateAndAdjustGain) { - const double factor = m_pReplayGain->get() / replayGain.getRatio(); - const double newPregain = m_pPreGain->get() * factor; - // There is a very slight chance that there will be a buffer call in between these sets. - // Therefore, we first adjust the control that is being lowered before the control - // that is being raised. Worst case, the volume goes down briefly before rectifying. - if (newPregain <= replayGain.getRatio()) { - m_pPreGain->set(newPregain); - setReplayGain(replayGain.getRatio()); - } else { - setReplayGain(replayGain.getRatio()); - m_pPreGain->set(newPregain); - } + // this may lead to an unexpected volume change. + if (m_pPlay->get() == 0.0) { + setReplayGain(replayGain.getRatio()); } else { m_replaygainPending = true; } } +void BaseTrackPlayerImpl::slotUpdateAndAdjustReplayGain(mixxx::ReplayGain replayGain) { + const double factor = m_pReplayGain->get() / replayGain.getRatio(); + const double newPregain = m_pPreGain->get() * factor; + + // There is a very slight chance that there will be a buffer call in between these sets. + // Therefore, we first adjust the control that is being lowered before the control + // that is being raised. Worst case, the volume goes down briefly before rectifying. + if (factor < 1.0) { + m_pPreGain->set(newPregain); + setReplayGain(replayGain.getRatio()); + } else { + setReplayGain(replayGain.getRatio()); + m_pPreGain->set(newPregain); + } +} + void BaseTrackPlayerImpl::slotSetTrackColor(const mixxx::RgbColor::optional_t& color) { m_pTrackColor->forceSet(trackColorToDouble(color)); } @@ -724,6 +739,21 @@ void BaseTrackPlayerImpl::slotShiftCuesMillisButton(double value, double millise slotShiftCuesMillis(milliseconds); } +void BaseTrackPlayerImpl::slotUpdateReplaygainFromDeckGain(double pressed) { + if (pressed <= 0) { + return; + } + if (!m_pLoadedTrack) { + return; + } + const double gain = m_pPreGain->get(); + // Gain is at unity already, ignore and return. + if (gain == 1.0) { + return; + } + m_pLoadedTrack->adjustReplayGainFromDeckGain(gain); +} + void BaseTrackPlayerImpl::setReplayGain(double value) { m_pReplayGain->set(value); m_replaygainPending = false; diff --git a/src/mixer/basetrackplayer.h b/src/mixer/basetrackplayer.h index 165dcd2c5e1a..d0509868cef3 100644 --- a/src/mixer/basetrackplayer.h +++ b/src/mixer/basetrackplayer.h @@ -81,8 +81,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { void slotCloneDeck() final; void slotTrackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack); void slotLoadFailed(TrackPointer pTrack, const QString& reason); - void slotSetReplayGain(mixxx::ReplayGain replayGain, - mixxx::ReplayGain::ReplayGainUpdateMode mode); + void slotSetReplayGain(mixxx::ReplayGain replayGain); + void slotUpdateAndAdjustReplayGain(mixxx::ReplayGain replayGain); void slotSetTrackColor(const mixxx::RgbColor::optional_t& color); void slotPlayToggled(double); @@ -98,6 +98,7 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { void slotWaveformZoomSetDefault(double pressed); void slotShiftCuesMillis(double milliseconds); void slotShiftCuesMillisButton(double value, double milliseconds); + void slotUpdateReplaygainFromDeckGain(double pressed); private: void setReplayGain(double value); @@ -143,6 +144,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { std::unique_ptr m_pShiftCuesLaterSmall; std::unique_ptr m_pShiftCues; + std::unique_ptr m_pUpdateReplayGainFromDeckGain; + parented_ptr m_pReplayGain; parented_ptr m_pPlay; parented_ptr m_pLowFilter; diff --git a/src/track/replaygain.h b/src/track/replaygain.h index a72af495c616..ea5360c2e552 100644 --- a/src/track/replaygain.h +++ b/src/track/replaygain.h @@ -99,11 +99,6 @@ class ReplayGain final { m_peak = normalizePeak(m_peak); } - enum ReplayGainUpdateMode { - UpdateWhenStopped, - UpdateAndAdjustGain - }; - private: double m_ratio; CSAMPLE m_peak; diff --git a/src/track/track.cpp b/src/track/track.cpp index 23f7b031f7d9..8012305a2ff6 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -207,7 +207,7 @@ void Track::replaceMetadataFromSource( emit keyChanged(); } if (oldReplayGain != newReplayGain) { - emit replayGainUpdated(newReplayGain, mixxx::ReplayGain::UpdateWhenStopped); + emit replayGainUpdated(newReplayGain); } if (colorModified) { emit colorUpdated(newColor); @@ -305,7 +305,7 @@ bool Track::replaceRecord( emit keyChanged(); } if (oldReplayGain != newReplayGain) { - emit replayGainUpdated(newReplayGain, mixxx::ReplayGain::UpdateWhenStopped); + emit replayGainUpdated(newReplayGain); } if (oldColor != newColor) { emit colorUpdated(newColor); @@ -324,25 +324,17 @@ void Track::setReplayGain(const mixxx::ReplayGain& replayGain) { auto locked = lockMutex(&m_qMutex); if (compareAndSet(m_record.refMetadata().refTrackInfo().ptrReplayGain(), replayGain)) { markDirtyAndUnlock(&locked); - emit replayGainUpdated(replayGain, mixxx::ReplayGain::UpdateWhenStopped); + emit replayGainUpdated(replayGain); } } -void Track::adjustReplayGainFromDeckGain(const QString& deckGroup) { +void Track::adjustReplayGainFromDeckGain(double gain) { QMutexLocker lock(&m_qMutex); mixxx::ReplayGain replayGain = m_record.getMetadata().getTrackInfo().getReplayGain(); - auto deckPregainCO = ControlProxy(deckGroup, "pregain"); - if (!deckPregainCO.valid()) { - qDebug() << "pregain CO not valid"; - return; - } - const double gain = deckPregainCO.get(); - qDebug() << "ADJUST REPLAYGAIN: old: " << replayGain.getRatio() - << " adjust " << gain << " so: " << replayGain.getRatio() * gain; - replayGain.setRatio(replayGain.getRatio() * gain); + replayGain.setRatio(gain * replayGain.getRatio()); if (compareAndSet(m_record.refMetadata().refTrackInfo().ptrReplayGain(), replayGain)) { markDirtyAndUnlock(&lock); - emit replayGainUpdated(replayGain, mixxx::ReplayGain::UpdateAndAdjustGain); + emit updateAndAdjustReplayGain(replayGain); } } diff --git a/src/track/track.h b/src/track/track.h index 1f174eafa30e..6e1e02939f13 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -144,8 +144,8 @@ class Track : public QObject { // Set ReplayGain void setReplayGain(const mixxx::ReplayGain&); - // Adjust ReplayGain by the value from the given deck's pregain knob. - void adjustReplayGainFromDeckGain(const QString& deckGroup); + // Adjust ReplayGain by multiplying the given gain amount. + void adjustReplayGainFromDeckGain(double); // Returns ReplayGain mixxx::ReplayGain getReplayGain() const; @@ -411,8 +411,10 @@ class Track : public QObject { void waveformSummaryUpdated(); void coverArtUpdated(); void beatsUpdated(); - void replayGainUpdated(mixxx::ReplayGain replayGain, - mixxx::ReplayGain::ReplayGainUpdateMode mode); + void replayGainUpdated(mixxx::ReplayGain replayGain); + // This signal indicates that ReplayGain is being adjusted, and pregains should be + // adjusted in the opposite direction to compensate (no audible change). + void updateAndAdjustReplayGain(const mixxx::ReplayGain&); void colorUpdated(const mixxx::RgbColor::optional_t& color); void cuesUpdated(); void analyzed(); diff --git a/src/widget/wtrackmenu.cpp b/src/widget/wtrackmenu.cpp index 802078db3080..f640197209a8 100644 --- a/src/widget/wtrackmenu.cpp +++ b/src/widget/wtrackmenu.cpp @@ -346,7 +346,7 @@ void WTrackMenu::createActions() { connect(m_pUpdateReplayGain, &QAction::triggered, this, - &WTrackMenu::slotUpdateReplaygain); + &WTrackMenu::slotUpdateReplaygainFromDeckGain); } if (featureIsEnabled(Feature::Color)) { @@ -843,10 +843,8 @@ TrackPointer WTrackMenu::getFirstTrackPointer() const { // Skip unavailable tracks } return TrackPointer(); - } else if (m_pTrack) { - return m_pTrack; } - return TrackPointer(); + return m_pTrack; } std::unique_ptr WTrackMenu::newTrackPointerIterator() const { @@ -917,15 +915,22 @@ class ImportMetadataFromFileTagsTrackPointerOperation : public mixxx::TrackPoint } // anonymous namespace -void WTrackMenu::slotUpdateReplaygain() { +void WTrackMenu::slotUpdateReplaygainFromDeckGain() { if (!m_pTrack) { qDebug() << "track pointer nullptr, returning"; return; } if (m_deckGroup.isEmpty()) { - qDebug() << "deck group not set"; + qDebug() << "Deck group not set"; + return; + } + + const double gain = ControlObject::get(ConfigKey(m_deckGroup, "pregain")); + // Gain is at unity already, ignore and return. + if (gain == 1.0) { + return; } - m_pTrack->adjustReplayGainFromDeckGain(m_deckGroup); + m_pTrack->adjustReplayGainFromDeckGain(gain); } void WTrackMenu::slotImportMetadataFromFileTags() { diff --git a/src/widget/wtrackmenu.h b/src/widget/wtrackmenu.h index 5e232d7c36e3..df556fb2c111 100644 --- a/src/widget/wtrackmenu.h +++ b/src/widget/wtrackmenu.h @@ -107,7 +107,7 @@ class WTrackMenu : public QMenu { void slotScaleBpm(mixxx::Beats::BpmScale scale); // Info and metadata - void slotUpdateReplaygain(); + void slotUpdateReplaygainFromDeckGain(); void slotShowDlgTagFetcher(); void slotImportMetadataFromFileTags(); void slotExportMetadataIntoFileTags(); From 6a2721ea3f1d84b23709678bd0f70a9df9e108d5 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Mon, 19 Jul 2021 19:41:09 -0400 Subject: [PATCH 4/8] ReplayGain: Use debug asserts for situations that shouldn't occur --- src/widget/wtrackmenu.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/widget/wtrackmenu.cpp b/src/widget/wtrackmenu.cpp index f640197209a8..a4894aa8def6 100644 --- a/src/widget/wtrackmenu.cpp +++ b/src/widget/wtrackmenu.cpp @@ -916,14 +916,8 @@ class ImportMetadataFromFileTagsTrackPointerOperation : public mixxx::TrackPoint } // anonymous namespace void WTrackMenu::slotUpdateReplaygainFromDeckGain() { - if (!m_pTrack) { - qDebug() << "track pointer nullptr, returning"; - return; - } - if (m_deckGroup.isEmpty()) { - qDebug() << "Deck group not set"; - return; - } + DEBUG_ASSERT(m_pTrack); + DEBUG_ASSERT(!m_deckGroup.isEmpty()); const double gain = ControlObject::get(ConfigKey(m_deckGroup, "pregain")); // Gain is at unity already, ignore and return. From a7a49d0e75b293d7303abdfc0449651282325b35 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Wed, 21 Jul 2021 15:02:26 -0400 Subject: [PATCH 5/8] Update ReplayGain: address small notes --- src/widget/wtrackmenu.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/widget/wtrackmenu.cpp b/src/widget/wtrackmenu.cpp index a4894aa8def6..9dc935d89534 100644 --- a/src/widget/wtrackmenu.cpp +++ b/src/widget/wtrackmenu.cpp @@ -916,8 +916,12 @@ class ImportMetadataFromFileTagsTrackPointerOperation : public mixxx::TrackPoint } // anonymous namespace void WTrackMenu::slotUpdateReplaygainFromDeckGain() { - DEBUG_ASSERT(m_pTrack); - DEBUG_ASSERT(!m_deckGroup.isEmpty()); + VERIFY_OR_DEBUG_ASSERT(m_pTrack) { + return; + } + VERIFY_OR_DEBUG_ASSERT(!m_deckGroup.isEmpty()) { + return; + } const double gain = ControlObject::get(ConfigKey(m_deckGroup, "pregain")); // Gain is at unity already, ignore and return. @@ -1751,7 +1755,7 @@ void WTrackMenu::slotPurge() { void WTrackMenu::clearTrackSelection() { m_pTrack = nullptr; - m_deckGroup.clear(); + m_deckGroup = QString(); m_trackIndexList.clear(); } From 460124bf9ce85010de544b0cc83e04b6a2e5e8b2 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Wed, 21 Jul 2021 22:18:26 -0400 Subject: [PATCH 6/8] Update Replaygain: normalize naming style Always refer to "pregain" instead of "deck gain". --- src/mixer/basetrackplayer.cpp | 16 ++++++++-------- src/mixer/basetrackplayer.h | 8 +++++--- src/track/track.cpp | 4 ++-- src/track/track.h | 4 ++-- src/widget/wtrackmenu.cpp | 6 +++--- src/widget/wtrackmenu.h | 2 +- 6 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index 8b0ad30de5b6..78e82e2ee570 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -193,12 +193,12 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl( m_pRateRatio = make_parented(getGroup(), "rate_ratio", this); m_pPitchAdjust = make_parented(getGroup(), "pitch_adjust", this); - m_pUpdateReplayGainFromDeckGain = std::make_unique( + m_pUpdateReplayGainFromPregain = std::make_unique( ConfigKey(getGroup(), "update_replaygain_from_pregain")); - connect(m_pUpdateReplayGainFromDeckGain.get(), + connect(m_pUpdateReplayGainFromPregain.get(), &ControlObject::valueChanged, this, - &BaseTrackPlayerImpl::slotUpdateReplaygainFromDeckGain); + &BaseTrackPlayerImpl::slotUpdateReplaygainFromPregain); } BaseTrackPlayerImpl::~BaseTrackPlayerImpl() { @@ -375,9 +375,9 @@ void BaseTrackPlayerImpl::connectLoadedTrack() { this, &BaseTrackPlayerImpl::slotSetReplayGain); connect(m_pLoadedTrack.get(), - &Track::updateAndAdjustReplayGain, + &Track::replayGainAdjusted, this, - &BaseTrackPlayerImpl::slotUpdateAndAdjustReplayGain); + &BaseTrackPlayerImpl::slotAdjustReplayGain); connect(m_pLoadedTrack.get(), &Track::colorUpdated, @@ -618,7 +618,7 @@ void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain) { } } -void BaseTrackPlayerImpl::slotUpdateAndAdjustReplayGain(mixxx::ReplayGain replayGain) { +void BaseTrackPlayerImpl::slotAdjustReplayGain(mixxx::ReplayGain replayGain) { const double factor = m_pReplayGain->get() / replayGain.getRatio(); const double newPregain = m_pPreGain->get() * factor; @@ -739,7 +739,7 @@ void BaseTrackPlayerImpl::slotShiftCuesMillisButton(double value, double millise slotShiftCuesMillis(milliseconds); } -void BaseTrackPlayerImpl::slotUpdateReplaygainFromDeckGain(double pressed) { +void BaseTrackPlayerImpl::slotUpdateReplaygainFromPregain(double pressed) { if (pressed <= 0) { return; } @@ -751,7 +751,7 @@ void BaseTrackPlayerImpl::slotUpdateReplaygainFromDeckGain(double pressed) { if (gain == 1.0) { return; } - m_pLoadedTrack->adjustReplayGainFromDeckGain(gain); + m_pLoadedTrack->adjustReplayGainFromPregain(gain); } void BaseTrackPlayerImpl::setReplayGain(double value) { diff --git a/src/mixer/basetrackplayer.h b/src/mixer/basetrackplayer.h index d0509868cef3..18e750f81ca9 100644 --- a/src/mixer/basetrackplayer.h +++ b/src/mixer/basetrackplayer.h @@ -82,7 +82,9 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { void slotTrackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack); void slotLoadFailed(TrackPointer pTrack, const QString& reason); void slotSetReplayGain(mixxx::ReplayGain replayGain); - void slotUpdateAndAdjustReplayGain(mixxx::ReplayGain replayGain); + // When the replaygain is adjusted, we modify the track pregain + // to compensate so there is no audible change in volume. + void slotAdjustReplayGain(mixxx::ReplayGain replayGain); void slotSetTrackColor(const mixxx::RgbColor::optional_t& color); void slotPlayToggled(double); @@ -98,7 +100,7 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { void slotWaveformZoomSetDefault(double pressed); void slotShiftCuesMillis(double milliseconds); void slotShiftCuesMillisButton(double value, double milliseconds); - void slotUpdateReplaygainFromDeckGain(double pressed); + void slotUpdateReplaygainFromPregain(double pressed); private: void setReplayGain(double value); @@ -144,7 +146,7 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { std::unique_ptr m_pShiftCuesLaterSmall; std::unique_ptr m_pShiftCues; - std::unique_ptr m_pUpdateReplayGainFromDeckGain; + std::unique_ptr m_pUpdateReplayGainFromPregain; parented_ptr m_pReplayGain; parented_ptr m_pPlay; diff --git a/src/track/track.cpp b/src/track/track.cpp index 8012305a2ff6..13af613fa4fa 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -328,13 +328,13 @@ void Track::setReplayGain(const mixxx::ReplayGain& replayGain) { } } -void Track::adjustReplayGainFromDeckGain(double gain) { +void Track::adjustReplayGainFromPregain(double gain) { QMutexLocker lock(&m_qMutex); mixxx::ReplayGain replayGain = m_record.getMetadata().getTrackInfo().getReplayGain(); replayGain.setRatio(gain * replayGain.getRatio()); if (compareAndSet(m_record.refMetadata().refTrackInfo().ptrReplayGain(), replayGain)) { markDirtyAndUnlock(&lock); - emit updateAndAdjustReplayGain(replayGain); + emit replayGainAdjusted(replayGain); } } diff --git a/src/track/track.h b/src/track/track.h index 6e1e02939f13..b0cf9b2a3838 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -145,7 +145,7 @@ class Track : public QObject { // Set ReplayGain void setReplayGain(const mixxx::ReplayGain&); // Adjust ReplayGain by multiplying the given gain amount. - void adjustReplayGainFromDeckGain(double); + void adjustReplayGainFromPregain(double); // Returns ReplayGain mixxx::ReplayGain getReplayGain() const; @@ -414,7 +414,7 @@ class Track : public QObject { void replayGainUpdated(mixxx::ReplayGain replayGain); // This signal indicates that ReplayGain is being adjusted, and pregains should be // adjusted in the opposite direction to compensate (no audible change). - void updateAndAdjustReplayGain(const mixxx::ReplayGain&); + void replayGainAdjusted(const mixxx::ReplayGain&); void colorUpdated(const mixxx::RgbColor::optional_t& color); void cuesUpdated(); void analyzed(); diff --git a/src/widget/wtrackmenu.cpp b/src/widget/wtrackmenu.cpp index 9dc935d89534..3e7ebb530991 100644 --- a/src/widget/wtrackmenu.cpp +++ b/src/widget/wtrackmenu.cpp @@ -346,7 +346,7 @@ void WTrackMenu::createActions() { connect(m_pUpdateReplayGain, &QAction::triggered, this, - &WTrackMenu::slotUpdateReplaygainFromDeckGain); + &WTrackMenu::slotUpdateReplaygainFromPregain); } if (featureIsEnabled(Feature::Color)) { @@ -915,7 +915,7 @@ class ImportMetadataFromFileTagsTrackPointerOperation : public mixxx::TrackPoint } // anonymous namespace -void WTrackMenu::slotUpdateReplaygainFromDeckGain() { +void WTrackMenu::slotUpdateReplaygainFromPregain() { VERIFY_OR_DEBUG_ASSERT(m_pTrack) { return; } @@ -928,7 +928,7 @@ void WTrackMenu::slotUpdateReplaygainFromDeckGain() { if (gain == 1.0) { return; } - m_pTrack->adjustReplayGainFromDeckGain(gain); + m_pTrack->adjustReplayGainFromPregain(gain); } void WTrackMenu::slotImportMetadataFromFileTags() { diff --git a/src/widget/wtrackmenu.h b/src/widget/wtrackmenu.h index df556fb2c111..243062001407 100644 --- a/src/widget/wtrackmenu.h +++ b/src/widget/wtrackmenu.h @@ -107,7 +107,7 @@ class WTrackMenu : public QMenu { void slotScaleBpm(mixxx::Beats::BpmScale scale); // Info and metadata - void slotUpdateReplaygainFromDeckGain(); + void slotUpdateReplaygainFromPregain(); void slotShowDlgTagFetcher(); void slotImportMetadataFromFileTags(); void slotExportMetadataIntoFileTags(); From 77e95fbfe083d1e756af6f58ff2672a045565a8b Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Fri, 23 Jul 2021 11:28:06 -0400 Subject: [PATCH 7/8] Update ReplayGain: Use consistent case for ReplayGain --- src/mixer/basetrackplayer.cpp | 4 ++-- src/mixer/basetrackplayer.h | 2 +- src/widget/wtrackmenu.cpp | 4 ++-- src/widget/wtrackmenu.h | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index 78e82e2ee570..b18d3eb1732a 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -198,7 +198,7 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl( connect(m_pUpdateReplayGainFromPregain.get(), &ControlObject::valueChanged, this, - &BaseTrackPlayerImpl::slotUpdateReplaygainFromPregain); + &BaseTrackPlayerImpl::slotUpdateReplayGainFromPregain); } BaseTrackPlayerImpl::~BaseTrackPlayerImpl() { @@ -739,7 +739,7 @@ void BaseTrackPlayerImpl::slotShiftCuesMillisButton(double value, double millise slotShiftCuesMillis(milliseconds); } -void BaseTrackPlayerImpl::slotUpdateReplaygainFromPregain(double pressed) { +void BaseTrackPlayerImpl::slotUpdateReplayGainFromPregain(double pressed) { if (pressed <= 0) { return; } diff --git a/src/mixer/basetrackplayer.h b/src/mixer/basetrackplayer.h index 18e750f81ca9..6746957cd589 100644 --- a/src/mixer/basetrackplayer.h +++ b/src/mixer/basetrackplayer.h @@ -100,7 +100,7 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { void slotWaveformZoomSetDefault(double pressed); void slotShiftCuesMillis(double milliseconds); void slotShiftCuesMillisButton(double value, double milliseconds); - void slotUpdateReplaygainFromPregain(double pressed); + void slotUpdateReplayGainFromPregain(double pressed); private: void setReplayGain(double value); diff --git a/src/widget/wtrackmenu.cpp b/src/widget/wtrackmenu.cpp index 3e7ebb530991..cf7e0993a767 100644 --- a/src/widget/wtrackmenu.cpp +++ b/src/widget/wtrackmenu.cpp @@ -346,7 +346,7 @@ void WTrackMenu::createActions() { connect(m_pUpdateReplayGain, &QAction::triggered, this, - &WTrackMenu::slotUpdateReplaygainFromPregain); + &WTrackMenu::slotUpdateReplayGainFromPregain); } if (featureIsEnabled(Feature::Color)) { @@ -915,7 +915,7 @@ class ImportMetadataFromFileTagsTrackPointerOperation : public mixxx::TrackPoint } // anonymous namespace -void WTrackMenu::slotUpdateReplaygainFromPregain() { +void WTrackMenu::slotUpdateReplayGainFromPregain() { VERIFY_OR_DEBUG_ASSERT(m_pTrack) { return; } diff --git a/src/widget/wtrackmenu.h b/src/widget/wtrackmenu.h index 243062001407..df627b32f4b8 100644 --- a/src/widget/wtrackmenu.h +++ b/src/widget/wtrackmenu.h @@ -107,7 +107,7 @@ class WTrackMenu : public QMenu { void slotScaleBpm(mixxx::Beats::BpmScale scale); // Info and metadata - void slotUpdateReplaygainFromPregain(); + void slotUpdateReplayGainFromPregain(); void slotShowDlgTagFetcher(); void slotImportMetadataFromFileTags(); void slotExportMetadataIntoFileTags(); @@ -212,7 +212,7 @@ class WTrackMenu : public QMenu { WCoverArtMenu* m_pCoverMenu{}; parented_ptr m_pSearchRelatedMenu; - // Update Replaygain from Track + // Update ReplayGain from Track QAction* m_pUpdateReplayGain{}; // Reload Track Metadata Action: From 0b624a059c0d4bae0e2489d718bc7b445342bea2 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Tue, 27 Jul 2021 10:48:11 -0400 Subject: [PATCH 8/8] Update Replaygain: Add a test --- src/mixer/basetrackplayer.cpp | 4 +--- src/test/replaygaintest.cpp | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index de57eaecb7b0..fc610b66cd20 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -195,9 +195,7 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl( m_pUpdateReplayGainFromPregain = std::make_unique( ConfigKey(getGroup(), "update_replaygain_from_pregain")); - connect(m_pUpdateReplayGainFromPregain.get(), - &ControlObject::valueChanged, - this, + m_pUpdateReplayGainFromPregain->connectValueChangeRequest(this, &BaseTrackPlayerImpl::slotUpdateReplayGainFromPregain); } diff --git a/src/test/replaygaintest.cpp b/src/test/replaygaintest.cpp index 3696f0fd8e0c..c9407470eabb 100644 --- a/src/test/replaygaintest.cpp +++ b/src/test/replaygaintest.cpp @@ -1,9 +1,10 @@ #include -#include "track/replaygain.h" - #include +#include "test/mockedenginebackendtest.h" +#include "track/replaygain.h" + namespace { class ReplayGainTest : public testing::Test { @@ -156,4 +157,33 @@ TEST_F(ReplayGainTest, NormalizePeak) { normalizePeak(mixxx::ReplayGain::kPeakClip + mixxx::ReplayGain::kPeakClip); } +class AdjustReplayGainTest : public MockedEngineBackendTest {}; + +TEST_F(AdjustReplayGainTest, AdjustReplayGainUpdatesPregain) { + // Initialize fake track replaygain so it's not zero. + mixxx::ReplayGain replayGain; + replayGain.setRatio(1.0); + m_pTrack1->setReplayGain(replayGain); + // Load the same track in decks 1 and 2 so we can see that the pregain is adjusted on both + // decks. + m_pMixerDeck2->slotLoadTrack(m_pTrack1, false); + // Because of this artificial process we have to manually set the replaygain CO for the second + // deck. + m_pMixerDeck2->slotSetReplayGain(replayGain); + + ControlObject::getControl(ConfigKey(m_sGroup1, "pregain"))->set(1.2); + ControlObject::getControl(ConfigKey(m_sGroup1, "update_replaygain_from_pregain"))->set(1.0); + + // The pregain value is folded into the replaygain value, and all pregains for all decks that + // have the same track loaded are adjusted so that the audible volume of the track does not + // change. + EXPECT_DOUBLE_EQ(1.0, ControlObject::getControl(ConfigKey(m_sGroup1, "pregain"))->get()); + EXPECT_NEAR(0.83333333, + ControlObject::getControl(ConfigKey(m_sGroup2, "pregain"))->get(), + .005); + EXPECT_DOUBLE_EQ(1.2, ControlObject::getControl(ConfigKey(m_sGroup1, "replaygain"))->get()); + EXPECT_DOUBLE_EQ(1.2, ControlObject::getControl(ConfigKey(m_sGroup2, "replaygain"))->get()); + EXPECT_DOUBLE_EQ(1.2, m_pTrack1->getReplayGain().getRatio()); +} + } // anonymous namespace