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.

Sign up for GitHub

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

Adding RadioLanewayCrossfade transition #13750

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Adding RadioLanewayCrossfade transition #13750

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2eb0ef6
Adding RadioLanewayCrossfade transition
davidlmorris Oct 10, 2024
8108686
Fix code style issues
davidlmorris Oct 10, 2024
eaa9f8e
Update autodjprocessor.cpp
davidlmorris Oct 10, 2024
134d99b
Update analyzersilence.h
davidlmorris Oct 10, 2024
1ddbc87
Missing case in cueinfo.cpp
davidlmorris Oct 10, 2024
1e2bab4
Update analyzersilence.cpp
davidlmorris Oct 10, 2024
56653d4
Update autodjprocessor.cpp
davidlmorris Oct 10, 2024
fde3728
Update autodjprocessor.cpp
davidlmorris Oct 10, 2024
854eb3e
Update cueinfo.cpp
davidlmorris Oct 10, 2024
a8465fb
Update dlgautodj.cpp
davidlmorris Oct 11, 2024
8f5dcd8
Code style format (yet again)
davidlmorris Oct 14, 2024
d25c655
Reverting the reverted code styling
davidlmorris Oct 14, 2024
8248b98
Update analyzersilence.cpp
davidlmorris Oct 17, 2024
e8e9854
Update analyzersilence.cpp
davidlmorris Oct 17, 2024
a37bbf8
Update analyzersilence.cpp
davidlmorris Oct 17, 2024
5c40e16
Merge branch 'mixxxdj:main' into AutoDj-Radio-Laneway-Transition
davidlmorris Oct 17, 2024
2129011
Update analyzersilence.h
davidlmorris Oct 17, 2024
6ac0c22
analyzersilence.cpp cf regression pull 13545
davidlmorris Oct 19, 2024
9147a85
Merge branch 'mixxxdj:main' into AutoDj-Radio-Laneway-Transition
davidlmorris Oct 19, 2024
f2cd61b
Update analyzersilence.cpp
davidlmorris Nov 18, 2024
2d36975
Update analyzersilence.cpp
davidlmorris Nov 18, 2024
ccb4f99
Comment changes
davidlmorris Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 108 additions & 12 deletions src/analyzer/analyzersilence.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,58 @@ namespace {
// verify that the track samples have not changed since the last analysis
constexpr CSAMPLE kSilenceThreshold = 0.001f; // -60 dB
// TODO: Change the above line to:
//constexpr CSAMPLE kSilenceThreshold = db2ratio(-60.0f);
// constexpr CSAMPLE kSilenceThreshold = db2ratio(-60.0f);

// These are in dBV expressed as Volts RMS (which seems, sensibly,
// the way Mixxx works).
// Don't change these to const as they are used only to feed the
// fade thresholds which are consts below themselves while we work
// out which one is best, and you'll just be adding to exe size.
Copy link
Member

Choose a reason for hiding this comment

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

Unused constexpr are not contribute to the EXE size. There might be only a "unused" warning.

Now we have unused macros instead. I would prefer to use constexpr and comment out the unused versions to mute warnings. This would be more expensive than the comment.

At least the comment needs to be corrected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The whole thing is a comment in case the chosen values are not the best. I should just delete the lot. I believe I have good values here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am a bit puzzled how this iteracts with manual set intros and outros.

You asked about this before. It does not interact or use manual set intros and outros cues at all. It runs as discussed in AutoDj: New transition mode 'Radio Laneway' #13716

I've yet to find a track where it isn't 'good enough', and in some cases does consistently better than me (where I might miss 1 in 10 cold endings), so I'm not sure I agree with you about not working with ambient noise at the beginning or end, but since I can't claim any great familiarity with club music I will leave that for you to decide.

Why not try it and see?

#define N_10DB_FADE_THRESHOLD 0.3162f
#define N_12DB_FADE_THRESHOLD 0.2511f
#define N_15DB_FADE_THRESHOLD 0.1778f
#define N_18DB_FADE_THRESHOLD 0.1259f
#define N_20DB_FADE_THRESHOLD 0.1f
#define N_24DB_FADE_THRESHOLD 0.0631f
#define N_25DB_FADE_THRESHOLD 0.0562f
#define N_27DB_FADE_THRESHOLD 0.0447f
#define N_30DB_FADE_THRESHOLD 0.0316f
#define N_40DB_FADE_THRESHOLD 0.01f
Copy link
Member

Choose a reason for hiding this comment

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

We need to consider how to handle future changes.
Since the thresholds are used fully automated without any user feedback a layer change if the values will mess up the database with old and new values.
Therfore a kind of "versioning" is require. That on case of a value change the old one found in database is no longer used.

We can easily add new cue type for this. But we need to prepare the code in a way that this is clear for future reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(My intent with those comments was to have a placeholder for the values while I played around. Assume those are deleted by the time someone is making a change here. They really are not necessary now)

But I understand the main point. It is the same problem as the -60db setting.

There would be a cost if we do a lot of processing every time a track is loaded. Should we have a Cue value that is simply a version flag associated with every track, which could be checked by a future version to fix any issues? No cue = 4.x or earlier?

For the user adjustment, I suggest that the next step would be to add some UI elements to make this all visible and allow the user adjust. We also discussed this. Happy to work on it as a next step (when I get time). But I think this might be a future version change, based on any perceived need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I sent that I realised 'Cue value' may not make sense. But at least have a flag with the track data that shows a version number. Or, alternatively have a cueinfo.version setting for each cue (though that smells of data duplication). After a very quick look at the code base - noting exceptions for things like "Serato's tag data", I realise even this might be problematic. So this too is also, worthy of a chat discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know what changes that have been requested remain incomplete at this stage.


constexpr CSAMPLE kFadeInThreshold = N_27DB_FADE_THRESHOLD;
constexpr CSAMPLE kFadeOutThreshold = N_12DB_FADE_THRESHOLD;

bool shouldAnalyze(TrackPointer pTrack) {
CuePointer pIntroCue = pTrack->findCueByType(mixxx::CueType::Intro);
CuePointer pOutroCue = pTrack->findCueByType(mixxx::CueType::Outro);
CuePointer pN60dBSound = pTrack->findCueByType(mixxx::CueType::N60dBSound);
CuePointer pFadeIn = pTrack->findCueByType(mixxx::CueType::FadeIn);
CuePointer pFadeOut = pTrack->findCueByType(mixxx::CueType::FadeOut);

if (!pIntroCue || !pOutroCue || !pN60dBSound || pN60dBSound->getLengthFrames() <= 0) {
if (!pFadeIn || !pFadeOut || !pIntroCue || !pOutroCue || !pN60dBSound ||
pN60dBSound->getLengthFrames() <= 0) {
return true;
}
return false;
}

template<typename Iterator>
Iterator first_sound(Iterator begin, Iterator end) {
return std::find_if(begin, end, [](const auto elem) {
return fabs(elem) >= kSilenceThreshold;
template<typename Iterator, typename Threshold>
Iterator find_first_above_threshold(Iterator begin,
Iterator end,
Threshold threshold) {
return std::find_if(begin, end, [threshold](const auto elem) {
return fabs(elem) >= threshold;
});
}

} // anonymous namespace

AnalyzerSilence::AnalyzerSilence(UserSettingsPointer pConfig)
: m_pConfig(pConfig),
m_framesProcessed(0),
m_signalStart(-1),
m_signalEnd(-1) {
m_signalEnd(-1),
m_fadeThresholdFadeInEnd(-1),
m_fadeThresholdFadeOutStart(-1) {
}

bool AnalyzerSilence::initialize(const AnalyzerTrack& track,
Expand All @@ -53,20 +78,47 @@ bool AnalyzerSilence::initialize(const AnalyzerTrack& track,
m_framesProcessed = 0;
m_signalStart = -1;
m_signalEnd = -1;
m_fadeThresholdFadeInEnd = -1;
m_fadeThresholdFadeOutStart = -1;
m_channelCount = channelCount;

return true;
}

// static
SINT AnalyzerSilence::findFirstSoundInChunk(std::span<const CSAMPLE> samples) {
return std::distance(samples.begin(), first_sound(samples.begin(), samples.end()));
return std::distance(samples.begin(),
find_first_above_threshold(
samples.begin(), samples.end(), kSilenceThreshold));
}

// static
SINT AnalyzerSilence::findLastSoundInChunk(std::span<const CSAMPLE> samples) {
// -1 is required, because the distance from the fist sample index (0) to crend() is 1,
SINT ret = std::distance(first_sound(samples.rbegin(), samples.rend()), samples.rend()) - 1;
SINT ret = std::distance(find_first_above_threshold(samples.rbegin(),
samples.rend(),
kSilenceThreshold),
samples.rend()) -
1;
return ret;
}

// Find the number of first sound sample where the sound is above kFadeInThreshold (-27db)
Copy link
Member

Choose a reason for hiding this comment

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

"Number" is ambiguous here better "index"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

SINT AnalyzerSilence::findLastFadeInChunk(std::span<const CSAMPLE> samples) {
SINT ret = std::distance(samples.begin(),
find_first_above_threshold(
samples.begin(), samples.end(), kFadeInThreshold));
return ret;
}

// Find the number of last sound sample where the sound is above kFadeOutThreshold (-12db)
SINT AnalyzerSilence::findFirstFadeOutChunk(std::span<const CSAMPLE> samples) {
// Note we are searching backwards from the end here.
SINT ret = std::distance(find_first_above_threshold(samples.rbegin(),
samples.rend(),
kFadeOutThreshold),
samples.rend()) -
1;
return ret;
}

Expand All @@ -93,10 +145,24 @@ bool AnalyzerSilence::processSamples(const CSAMPLE* pIn, SINT count) {
m_signalStart = m_framesProcessed + firstSoundSample / m_channelCount;
}
}
if (m_signalStart >= 0) {

if (m_fadeThresholdFadeInEnd < 0) {
const SINT lastSampleOfFadeIn = findLastFadeInChunk(samples);
if (lastSampleOfFadeIn < count) {
m_fadeThresholdFadeInEnd = m_framesProcessed + (lastSampleOfFadeIn / m_channelCount);
}
}
if (m_fadeThresholdFadeInEnd >= 0) {
const SINT lasttSampleBeforeFadeOut = findFirstFadeOutChunk(samples);
if (lasttSampleBeforeFadeOut >= 0) {
m_fadeThresholdFadeOutStart = m_framesProcessed +
(lasttSampleBeforeFadeOut / m_channelCount) + 1;
}
}
if (m_fadeThresholdFadeOutStart >= 0) {
const SINT lastSoundSample = findLastSoundInChunk(samples);
if (lastSoundSample >= 0) {
m_signalEnd = m_framesProcessed + lastSoundSample / m_channelCount + 1;
m_signalEnd = m_framesProcessed + (lastSoundSample / m_channelCount) + 1;
}
}

Expand Down Expand Up @@ -136,6 +202,36 @@ void AnalyzerSilence::storeResults(TrackPointer pTrack) {

setupMainAndIntroCue(pTrack.get(), firstSoundPosition, m_pConfig.data());
setupOutroCue(pTrack.get(), lastSoundPosition);

if (m_fadeThresholdFadeInEnd < 0) {
m_fadeThresholdFadeInEnd = 0;
}
if (m_fadeThresholdFadeOutStart < 0) {
m_fadeThresholdFadeOutStart = m_framesProcessed;
}
const auto fadeInEndPosition = mixxx::audio::FramePos(m_fadeThresholdFadeInEnd);
CuePointer pFadeIn = pTrack->findCueByType(mixxx::CueType::FadeIn);
if (pFadeIn == nullptr) {
pFadeIn = pTrack->createAndAddCue(
mixxx::CueType::FadeIn,
Cue::kNoHotCue,
firstSoundPosition,
fadeInEndPosition);
} else {
pFadeIn->setStartAndEndPosition(firstSoundPosition, fadeInEndPosition);
}

const auto fadeOutStartPosition = mixxx::audio::FramePos(m_fadeThresholdFadeOutStart);
CuePointer pFadeOut = pTrack->findCueByType(mixxx::CueType::FadeOut);
if (pFadeOut == nullptr) {
pFadeOut = pTrack->createAndAddCue(
mixxx::CueType::FadeOut,
Cue::kNoHotCue,
fadeOutStartPosition,
lastSoundPosition);
} else {
pFadeOut->setStartAndEndPosition(fadeOutStartPosition, lastSoundPosition);
}
}

// static
Expand Down
11 changes: 11 additions & 0 deletions src/analyzer/analyzersilence.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ class AnalyzerSilence : public Analyzer {
UserSettings* pConfig);
static void setupOutroCue(Track* pTrack, mixxx::audio::FramePos lastSoundPosition);

/// returns the index of the first sample in the buffer that is above the
/// fade in threshold (e.g. -27 dB).
static SINT findLastFadeInChunk(std::span<const CSAMPLE> samples);

/// returns the index of the last sample in the buffer that is
/// above the fade out threshold (e.g. -12 dB) or samples.size() if no
/// sample is found
static SINT findFirstFadeOutChunk(std::span<const CSAMPLE> samples);

/// returns the index of the first sample in the buffer that is above -60 dB
/// or samples.size() if no sample is found
static SINT findFirstSoundInChunk(std::span<const CSAMPLE> samples);
Expand All @@ -56,4 +65,6 @@ class AnalyzerSilence : public Analyzer {
SINT m_framesProcessed;
SINT m_signalStart;
SINT m_signalEnd;
SINT m_fadeThresholdFadeInEnd;
SINT m_fadeThresholdFadeOutStart;
};
Loading
Loading