Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust ReplayGain: Allow user to update the replaygain value based on a deck pregain value. #4031

Merged
merged 10 commits into from
Jul 28, 2021
44 changes: 43 additions & 1 deletion src/mixer/basetrackplayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl(

m_pRateRatio = make_parented<ControlProxy>(getGroup(), "rate_ratio", this);
m_pPitchAdjust = make_parented<ControlProxy>(getGroup(), "pitch_adjust", this);

m_pUpdateReplayGainFromDeckGain = std::make_unique<ControlPushButton>(
ConfigKey(getGroup(), "update_replaygain_from_pregain"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite a verbose name for a ControlObject... how about just update_replaygain... or set_replaygain? store_replaygain?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the name. Our other CO names are often extremely ambiguous (so this PR introduces inconsistency 😅). You suggestions sound like I can set a the replaygain value directly, not like a button CO that takes only 1 and 0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I don't know how to make it shorter and include the ideas of both the deck pregain and the track replaygain. "apply_pregain_to_replaygain" ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how to remove any more words without making it confusing, and I don't think we have any limitations on CO name length

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't thought of any better name, so... 🤷 I won't object to merging this over finding a better name.

connect(m_pUpdateReplayGainFromDeckGain.get(),
&ControlObject::valueChanged,
this,
&BaseTrackPlayerImpl::slotUpdateReplaygainFromDeckGain);
}

BaseTrackPlayerImpl::~BaseTrackPlayerImpl() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -599,14 +610,30 @@ void BaseTrackPlayerImpl::slotCloneChannel(EngineChannel* pChannel) {

void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain) {
// Do not change replay gain when track is playing because
// this may lead to an unexpected volume change
// this may lead to an unexpected volume change.
if (m_pPlay->get() == 0.0) {
setReplayGain(replayGain.getRatio());
} else {
m_replaygainPending = true;
daschuer marked this conversation as resolved.
Show resolved Hide resolved
}
}

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));
}
Expand Down Expand Up @@ -712,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;
Expand Down
4 changes: 4 additions & 0 deletions src/mixer/basetrackplayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ 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);
void slotSetTrackColor(const mixxx::RgbColor::optional_t& color);
void slotPlayToggled(double);

Expand All @@ -97,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);
Expand Down Expand Up @@ -142,6 +144,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
std::unique_ptr<ControlPushButton> m_pShiftCuesLaterSmall;
std::unique_ptr<ControlObject> m_pShiftCues;

std::unique_ptr<ControlObject> m_pUpdateReplayGainFromDeckGain;

parented_ptr<ControlProxy> m_pReplayGain;
parented_ptr<ControlProxy> m_pPlay;
parented_ptr<ControlProxy> m_pLowFilter;
Expand Down
4 changes: 2 additions & 2 deletions src/track/replaygain.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class ReplayGain final {
m_peak = normalizePeak(m_peak);
}

private:
private:
double m_ratio;
CSAMPLE m_peak;
};
Expand All @@ -119,7 +119,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)
10 changes: 10 additions & 0 deletions src/track/track.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,16 @@ void Track::setReplayGain(const mixxx::ReplayGain& replayGain) {
}
}

void Track::adjustReplayGainFromDeckGain(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);
}
}

mixxx::Bpm Track::getBpmWhileLocked() const {
// BPM values must be synchronized at all times!
DEBUG_ASSERT(m_record.getMetadata().getTrackInfo().getBpm() == getBeatsPointerBpm(m_pBeats));
Expand Down
5 changes: 5 additions & 0 deletions src/track/track.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ class Track : public QObject {

// Set ReplayGain
void setReplayGain(const mixxx::ReplayGain&);
// Adjust ReplayGain by multiplying the given gain amount.
void adjustReplayGainFromDeckGain(double);
// Returns ReplayGain
mixxx::ReplayGain getReplayGain() const;

Expand Down Expand Up @@ -410,6 +412,9 @@ class Track : public QObject {
void coverArtUpdated();
void beatsUpdated();
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&);
ywwg marked this conversation as resolved.
Show resolved Hide resolved
void colorUpdated(const mixxx::RgbColor::optional_t& color);
void cuesUpdated();
void analyzed();
Expand Down
98 changes: 58 additions & 40 deletions src/widget/wtrackmenu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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::slotUpdateReplaygainFromDeckGain);
}

if (featureIsEnabled(Feature::Color)) {
ColorPaletteSettings colorPaletteSettings(m_pConfig);
m_pColorPickerAction = new WColorPickerAction(WColorPicker::Option::AllowNoColor,
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -508,10 +521,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;
Expand All @@ -536,13 +547,10 @@ std::optional<std::optional<mixxx::RgbColor>> 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);
}
Expand Down Expand Up @@ -605,7 +613,7 @@ CoverInfo WTrackMenu::getCoverInfoOfLastTrack() const {
.toString();
return coverInfo;
} else {
return m_trackPointerList.last()->getCoverInfoWithLocation();
return m_pTrack->getCoverInfoWithLocation();
}
}

Expand Down Expand Up @@ -706,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());
Expand Down Expand Up @@ -741,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.
Expand All @@ -755,7 +767,8 @@ void WTrackMenu::loadTrack(
if (!pTrack) {
return;
}
m_trackPointerList = TrackPointerList{pTrack};
m_pTrack = pTrack;
m_deckGroup = deckGroup;
updateMenus();
}

Expand Down Expand Up @@ -788,9 +801,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);
}
Expand All @@ -812,15 +824,11 @@ QList<TrackRef> 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(
ywwg marked this conversation as resolved.
Show resolved Hide resolved
m_pTrack->getFileInfo(),
m_pTrack->getId());
trackRefs.push_back(std::move(trackRef));
}
return trackRefs;
}
Expand All @@ -835,13 +843,8 @@ TrackPointer WTrackMenu::getFirstTrackPointer() const {
// Skip unavailable tracks
}
return TrackPointer();
uklotzde marked this conversation as resolved.
Show resolved Hide resolved
} else {
if (m_trackPointerList.isEmpty()) {
return TrackPointer();
}
DEBUG_ASSERT(m_trackPointerList.first());
return m_trackPointerList.first();
}
return m_pTrack;
}

std::unique_ptr<mixxx::TrackPointerIterator> WTrackMenu::newTrackPointerIterator() const {
Expand All @@ -854,13 +857,11 @@ std::unique_ptr<mixxx::TrackPointerIterator> WTrackMenu::newTrackPointerIterator
return std::make_unique<mixxx::TrackPointerModelIterator>(
m_pTrackModel,
m_trackIndexList);
} else {
if (m_trackPointerList.isEmpty()) {
return nullptr;
}
} else if (m_pTrack) {
return std::make_unique<mixxx::TrackPointerListIterator>(
m_trackPointerList);
TrackPointerList{m_pTrack});
}
return nullptr;
}

int WTrackMenu::applyTrackPointerOperation(
Expand Down Expand Up @@ -914,6 +915,22 @@ class ImportMetadataFromFileTagsTrackPointerOperation : public mixxx::TrackPoint

} // anonymous namespace

void WTrackMenu::slotUpdateReplaygainFromDeckGain() {
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.
if (gain == 1.0) {
return;
}
m_pTrack->adjustReplayGainFromDeckGain(gain);
}

void WTrackMenu::slotImportMetadataFromFileTags() {
const auto progressLabelText =
tr("Importing metadata of %n track(s) from file tags", "", getTrackCount());
Expand Down Expand Up @@ -1597,7 +1614,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();
}
Expand All @@ -1621,7 +1638,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();
}
Expand Down Expand Up @@ -1737,7 +1754,8 @@ void WTrackMenu::slotPurge() {
}

void WTrackMenu::clearTrackSelection() {
m_trackPointerList.clear();
m_pTrack = nullptr;
m_deckGroup = QString();
m_trackIndexList.clear();
}

Expand Down
Loading