Skip to content

Commit

Permalink
Merge pull request #4245 from uklotzde/lp1934785-soundsourceffmpeg
Browse files Browse the repository at this point in the history
lp1934785 SoundSourceFFmpeg: Ignore inaudible samples before start of stream
  • Loading branch information
Holzhaus authored Sep 3, 2021
2 parents 6cd39a5 + a509808 commit 40085a3
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* Fix performance issue on AArch64 by enabling flush-to-zero for floating-point arithmetic [#4144](https://github.com/mixxxdj/mixxx/pull/4144)
* Fix custom key notation not restored correctly after restart [#4136](https://github.com/mixxxdj/mixxx/pull/4136)
* Traktor S3: Disable scratch when switching decks to prevent locked scratch issue [#4073](https://github.com/mixxxdj/mixxx/pull/4073)
* FFmpeg: Ignore inaudible samples before start of stream [#4245](https://github.com/mixxxdj/mixxx/pull/4245)

### Packaging

Expand Down
19 changes: 19 additions & 0 deletions src/sources/readaheadframebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,17 @@
#include "util/logger.h"
#include "util/sample.h"

// Override or set to `true` to enable verbose debug logging.
#if !defined(VERBOSE_DEBUG_LOG)
#define VERBOSE_DEBUG_LOG false
#endif

// Override or set to `true` to break with a debug assertion
// if an overlap or gap in the audio stream has been detected.
#if !defined(DEBUG_ASSERT_ON_DISCONTINUITIES)
#define DEBUG_ASSERT_ON_DISCONTINUITIES false
#endif

namespace mixxx {

namespace {
Expand Down Expand Up @@ -135,6 +142,9 @@ ReadableSampleFrames ReadAheadFrameBuffer::fillBuffer(
<< bufferedRange()
<< "and input buffer"
<< inputRange;
#if DEBUG_ASSERT_ON_DISCONTINUITIES
DEBUG_ASSERT(!"Unexpected gap");
#endif
switch (discontinuityGapMode) {
case DiscontinuityGapMode::Skip:
reset(inputRange.start());
Expand Down Expand Up @@ -314,6 +324,9 @@ WritableSampleFrames ReadAheadFrameBuffer::consumeAndFillBuffer(
<< outputRange
<< "with input buffer"
<< inputRange;
#if DEBUG_ASSERT_ON_DISCONTINUITIES
DEBUG_ASSERT(!"Unexpected overlap");
#endif
switch (discontinuityOverlapMode) {
case DiscontinuityOverlapMode::Ignore:
break;
Expand Down Expand Up @@ -345,6 +358,9 @@ WritableSampleFrames ReadAheadFrameBuffer::consumeAndFillBuffer(
<< bufferedRange()
<< "with input buffer"
<< inputRange;
#if DEBUG_ASSERT_ON_DISCONTINUITIES
DEBUG_ASSERT(!"Unexpected overlap");
#endif
switch (discontinuityOverlapMode) {
case DiscontinuityOverlapMode::Ignore:
break;
Expand Down Expand Up @@ -397,6 +413,9 @@ WritableSampleFrames ReadAheadFrameBuffer::consumeAndFillBuffer(
<< outputRange
<< "and input buffer"
<< inputRange;
#if DEBUG_ASSERT_ON_DISCONTINUITIES
DEBUG_ASSERT(!"Unexpected gap");
#endif
switch (discontinuityGapMode) {
case DiscontinuityGapMode::Skip:
break;
Expand Down
35 changes: 31 additions & 4 deletions src/sources/soundsourceffmpeg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ constexpr int64_t kavStreamDecoderFrameDelayAAC = 2112;
// Use 0-based sample frame indexing
constexpr SINT kMinFrameIndex = 0;

constexpr SINT kSamplesPerMP3Frame = 1152;
constexpr SINT kMaxSamplesPerMP3Frame = 1152;

const Logger kLogger("SoundSourceFFmpeg");

Expand Down Expand Up @@ -106,7 +106,7 @@ inline int64_t getStreamStartTime(const AVStream& avStream) {
// using the default start time.
// Not all M4A files encode the start_time correctly, e.g.
// the test file cover-test-itunes-12.7.0-aac.m4a has a valid
// start_time of 0. Unfortunately, this special case is cannot
// start_time of 0. Unfortunately, this special case cannot be
// detected and compensated.
start_time = math_max(kavStreamDefaultStartTime, kavStreamDecoderFrameDelayAAC);
break;
Expand Down Expand Up @@ -135,6 +135,7 @@ inline int64_t getStreamEndTime(const AVStream& avStream) {
}

inline SINT convertStreamTimeToFrameIndex(const AVStream& avStream, int64_t pts) {
DEBUG_ASSERT(pts != AV_NOPTS_VALUE);
// getStreamStartTime(avStream) -> 1st audible frame at kMinFrameIndex
return kMinFrameIndex +
av_rescale_q(
Expand Down Expand Up @@ -185,7 +186,7 @@ SINT getStreamSeekPrerollFrameCount(const AVStream& avStream) {
// slight deviations from the exact signal!
DEBUG_ASSERT(avStream.codecpar->channels <= 2);
const SINT mp3SeekPrerollFrameCount =
9 * (kSamplesPerMP3Frame / avStream.codecpar->channels);
9 * (kMaxSamplesPerMP3Frame / avStream.codecpar->channels);
return math_max(mp3SeekPrerollFrameCount, defaultSeekPrerollFrameCount);
}
case AV_CODEC_ID_AAC:
Expand Down Expand Up @@ -1051,12 +1052,33 @@ ReadableSampleFrames SoundSourceFFmpeg::readSampleFramesClamped(
#if VERBOSE_DEBUG_LOG
avTrace("Received decoded frame", *m_pavDecodedFrame);
#endif
DEBUG_ASSERT(m_pavDecodedFrame->pts != AV_NOPTS_VALUE);
VERIFY_OR_DEBUG_ASSERT(
(m_pavDecodedFrame->flags &
(AV_FRAME_FLAG_CORRUPT |
AV_FRAME_FLAG_DISCARD)) == 0) {
av_frame_unref(m_pavDecodedFrame);
continue;
}
const auto decodedFrameCount = m_pavDecodedFrame->nb_samples;
DEBUG_ASSERT(decodedFrameCount > 0);
auto streamFrameIndex =
convertStreamTimeToFrameIndex(
*m_pavStream, m_pavDecodedFrame->pts);
// Only audible samples are counted, i.e. any inaudible aka
// "priming" samples are not included in nb_samples!
// https://bugs.launchpad.net/mixxx/+bug/1934785
if (streamFrameIndex < kMinFrameIndex) {
#if VERBOSE_DEBUG_LOG
const auto inaudibleFrameCountUntilStartOfStream =
kMinFrameIndex - streamFrameIndex;
kLogger.debug()
<< "Skipping"
<< inaudibleFrameCountUntilStartOfStream
<< "inaudible sample frames before the start of the stream";
#endif
streamFrameIndex = kMinFrameIndex;
}
DEBUG_ASSERT(streamFrameIndex >= kMinFrameIndex);
decodedFrameRange = IndexRange::forward(
streamFrameIndex,
decodedFrameCount);
Expand Down Expand Up @@ -1203,6 +1225,11 @@ ReadableSampleFrames SoundSourceFFmpeg::readSampleFramesClamped(
// Housekeeping before next decoding iteration
av_frame_unref(m_pavDecodedFrame);
av_frame_unref(m_pavResampledFrame);

// The first loop condition (see below) should always be true
// and has only been added to prevent infinite looping in case
// of unexpected result values.
DEBUG_ASSERT(avcodec_receive_frame_result == 0);
} while (avcodec_receive_frame_result == 0 &&
m_frameBuffer.isValid());
}
Expand Down

0 comments on commit 40085a3

Please sign in to comment.