Skip to content

Commit

Permalink
Merge pull request #626 from uklotzde/CachingReaderAvoidUnnecessarySe…
Browse files Browse the repository at this point in the history 
…eking

Fix outstanding SoundSource issues
  • Loading branch information
@daschuer
daschuer committed Jun 30, 2015
2 parents afb21ed + e0f217e commit c390cba
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 60 deletions.
95 changes: 53 additions & 42 deletions plugins/soundsourcem4a/soundsourcem4a.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ const MP4SampleId kNumberOfPrefetchSampleBlocks = 3;
// Searches for the first audio track in the MP4 file that
// suits our needs.
MP4TrackId findFirstAudioTrackId(MP4FileHandle hFile) {
const MP4TrackId maxTrackId = MP4GetNumberOfTracks(hFile, NULL, 0);
const MP4TrackId maxTrackId = MP4GetNumberOfTracks(hFile, nullptr, 0);
for (MP4TrackId trackId = 1; trackId <= maxTrackId; ++trackId) {
const char* trackType = MP4GetTrackType(hFile, trackId);
if ((NULL == trackType) || !MP4_IS_AUDIO_TRACK_TYPE(trackType)) {
if ((nullptr == trackType) || !MP4_IS_AUDIO_TRACK_TYPE(trackType)) {
continue;
}
const char* mediaDataName = MP4GetTrackMediaDataName(hFile, trackId);
if (0 != strcasecmp(mediaDataName, "mp4a")) {
if ((nullptr == mediaDataName) || (0 != strcasecmp(mediaDataName, "mp4a"))) {
continue;
}
const u_int8_t audioType = MP4GetTrackEsdsObjectTypeId(hFile, trackId);
Expand Down Expand Up @@ -99,7 +99,7 @@ SoundSourceM4A::SoundSourceM4A(const QUrl& url)
m_curSampleBlockId(MP4_INVALID_SAMPLE_ID),
m_inputBufferLength(0),
m_inputBufferOffset(0),
m_hDecoder(NULL),
m_hDecoder(nullptr),
m_curFrameIndex(getMinFrameIndex()) {
}

Expand Down Expand Up @@ -140,7 +140,7 @@ Result SoundSourceM4A::tryOpen(const AudioSourceConfig& audioSrcCfg) {
m_trackId);
m_inputBuffer.resize(maxSampleBlockInputSize, 0);

DEBUG_ASSERT(NULL == m_hDecoder); // not already opened
DEBUG_ASSERT(nullptr == m_hDecoder); // not already opened
m_hDecoder = NeAACDecOpen();
if (!m_hDecoder) {
qWarning() << "Failed to open the AAC decoder!";
Expand All @@ -162,7 +162,7 @@ Result SoundSourceM4A::tryOpen(const AudioSourceConfig& audioSrcCfg) {
return ERR;
}

u_int8_t* configBuffer = NULL;
u_int8_t* configBuffer = nullptr;
u_int32_t configBufferSize = 0;
if (!MP4GetTrackESConfiguration(m_hFile, m_trackId, &configBuffer,
&configBufferSize)) {
Expand Down Expand Up @@ -206,7 +206,7 @@ Result SoundSourceM4A::tryOpen(const AudioSourceConfig& audioSrcCfg) {
void SoundSourceM4A::close() {
if (m_hDecoder) {
NeAACDecClose(m_hDecoder);
m_hDecoder = NULL;
m_hDecoder = nullptr;
}
if (MP4_INVALID_FILE_HANDLE != m_hFile) {
MP4Close(m_hFile);
Expand Down Expand Up @@ -235,44 +235,55 @@ void SoundSourceM4A::restartDecoding(MP4SampleId sampleBlockId) {
}

SINT SoundSourceM4A::seekSampleFrame(SINT frameIndex) {
DEBUG_ASSERT(isValidFrameIndex(m_curFrameIndex));
DEBUG_ASSERT(isValidFrameIndex(frameIndex));

if (m_curFrameIndex != frameIndex) {
MP4SampleId sampleBlockId = kSampleBlockIdMin
+ (frameIndex / kFramesPerSampleBlock);
DEBUG_ASSERT(isValidSampleBlockId(sampleBlockId));
if ((frameIndex < m_curFrameIndex) || // seeking backwards?
!isValidSampleBlockId(m_curSampleBlockId) || // invalid seek position?
(sampleBlockId
> (m_curSampleBlockId + kNumberOfPrefetchSampleBlocks))) { // jumping forward?
// Restart decoding one or more blocks of samples backwards
// from the calculated starting block to avoid audible glitches.
// Implementation note: The type MP4SampleId is unsigned so we
// need to be careful when subtracting!
if ((kSampleBlockIdMin + kNumberOfPrefetchSampleBlocks)
< sampleBlockId) {
sampleBlockId -= kNumberOfPrefetchSampleBlocks;
} else {
sampleBlockId = kSampleBlockIdMin;
}
restartDecoding(sampleBlockId);
DEBUG_ASSERT(m_curSampleBlockId == sampleBlockId);
}
// Decoding starts before the actual target position
DEBUG_ASSERT(m_curFrameIndex <= frameIndex);
// Prefetch (decode and discard) all samples up to the target position
const SINT prefetchFrameCount = frameIndex - m_curFrameIndex;
const SINT skipFrameCount =
skipSampleFrames(prefetchFrameCount);
DEBUG_ASSERT(skipFrameCount <= prefetchFrameCount);
if (skipFrameCount != prefetchFrameCount) {
qWarning()
<< "Failed to skip over prefetched sample frames after seeking @"
<< m_curFrameIndex;
return m_curFrameIndex; // abort
// Handle trivial case
if (m_curFrameIndex == frameIndex) {
// Nothing to do
return m_curFrameIndex;
}
// Handle edge case
if (getMaxFrameIndex() <= frameIndex) {
// EOF reached
m_curFrameIndex = getMaxFrameIndex();
return m_curFrameIndex;
}

MP4SampleId sampleBlockId = kSampleBlockIdMin
+ (frameIndex / kFramesPerSampleBlock);
DEBUG_ASSERT(isValidSampleBlockId(sampleBlockId));
if ((frameIndex < m_curFrameIndex) || // seeking backwards?
!isValidSampleBlockId(m_curSampleBlockId) || // invalid seek position?
(sampleBlockId
> (m_curSampleBlockId + kNumberOfPrefetchSampleBlocks))) { // jumping forward?
// Restart decoding one or more blocks of samples backwards
// from the calculated starting block to avoid audible glitches.
// Implementation note: The type MP4SampleId is unsigned so we
// need to be careful when subtracting!
if ((kSampleBlockIdMin + kNumberOfPrefetchSampleBlocks)
< sampleBlockId) {
sampleBlockId -= kNumberOfPrefetchSampleBlocks;
} else {
sampleBlockId = kSampleBlockIdMin;
}
restartDecoding(sampleBlockId);
DEBUG_ASSERT(m_curSampleBlockId == sampleBlockId);
}

// Decoding starts before the actual target position
DEBUG_ASSERT(m_curFrameIndex <= frameIndex);

// Skip (= decode and discard) all samples up to the target position
const SINT prefetchFrameCount = frameIndex - m_curFrameIndex;
const SINT skipFrameCount = skipSampleFrames(prefetchFrameCount);
DEBUG_ASSERT(skipFrameCount <= prefetchFrameCount);
if (skipFrameCount < prefetchFrameCount) {
qWarning() << "Failed to prefetch sample data while seeking"
<< skipFrameCount << "<" << prefetchFrameCount;
}
DEBUG_ASSERT(m_curFrameIndex == frameIndex);

DEBUG_ASSERT(isValidFrameIndex(m_curFrameIndex));
return m_curFrameIndex;
}

Expand Down Expand Up @@ -315,7 +326,7 @@ SINT SoundSourceM4A::readSampleFrames(
u_int32_t inputBufferLength = m_inputBuffer.size(); // in/out parameter
if (!MP4ReadSample(m_hFile, m_trackId, m_curSampleBlockId,
&pInputBuffer, &inputBufferLength,
NULL, NULL, NULL, NULL)) {
nullptr, nullptr, nullptr, nullptr)) {
qWarning()
<< "Failed to read MP4 input data for sample block"
<< m_curSampleBlockId << "(" << "min ="
Expand Down
11 changes: 7 additions & 4 deletions src/cachingreader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ CachingReader::CachingReader(QString group,
for (int i = 0; i < maximumChunksInMemory; ++i) {
Chunk* c = new Chunk;
c->chunk_number = -1;
c->frameCount = 0;
c->frameCountRead = 0;
c->frameCountTotal = 0;
c->stereoSamples = bufferStart;
c->next_lru = NULL;
c->prev_lru = NULL;
Expand Down Expand Up @@ -146,7 +147,8 @@ void CachingReader::freeChunk(Chunk* pChunk) {
m_mruChunk = removeFromLRUList(pChunk, m_mruChunk);
pChunk->state = Chunk::FREE;
pChunk->chunk_number = -1;
pChunk->frameCount = 0;
pChunk->frameCountRead = 0;
pChunk->frameCountTotal = 0;
m_freeChunks.push_back(pChunk);
}

Expand All @@ -167,7 +169,8 @@ void CachingReader::freeAllChunks() {
if (pChunk->state != Chunk::FREE) {
pChunk->state = Chunk::FREE;
pChunk->chunk_number = -1;
pChunk->frameCount = 0;
pChunk->frameCountRead = 0;
pChunk->frameCountTotal = 0;
pChunk->next_lru = NULL;
pChunk->prev_lru = NULL;
m_freeChunks.append(pChunk);
Expand Down Expand Up @@ -381,7 +384,7 @@ int CachingReader::read(int sample, int num_samples, CSAMPLE* buffer) {

int chunk_start_frame = CachingReaderWorker::frameForChunk(chunk_num);
const int chunk_frame_offset = current_frame - chunk_start_frame;
int chunk_remaining_frames = current->frameCount - chunk_frame_offset;
int chunk_remaining_frames = current->frameCountTotal - chunk_frame_offset;

// More sanity checks
if (current_frame < chunk_start_frame) {
Expand Down
19 changes: 15 additions & 4 deletions src/cachingreaderworker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "cachingreaderworker.h"
#include "trackinfoobject.h"
#include "sampleutil.h"
#include "soundsourceproxy.h"
#include "util/compatibility.h"
#include "util/event.h"
Expand Down Expand Up @@ -45,7 +46,8 @@ void CachingReaderWorker::processChunkReadRequest(

// Initialize the output parameter
update->chunk = request->chunk;
update->chunk->frameCount = 0;
update->chunk->frameCountRead = 0;
update->chunk->frameCountTotal = 0;

const int chunk_number = request->chunk->chunk_number;
if (!m_pAudioSource || chunk_number < 0) {
Expand All @@ -62,12 +64,17 @@ void CachingReaderWorker::processChunkReadRequest(
update->status = CHUNK_READ_INVALID;
return;
}
if (m_pAudioSource->getMaxFrameIndex() <= chunkFrameIndex) {
// No more data available for reading
update->status = CHUNK_READ_EOF;
return;
}

const SINT seekFrameIndex =
m_pAudioSource->seekSampleFrame(chunkFrameIndex);
if (seekFrameIndex != chunkFrameIndex) {
// Failed to seek to the requested index.
// Corrupt file? -> Stop reading!
// Failed to seek to the requested index. The file might
// be corrupt and decoding should be aborted.
qWarning() << "Failed to seek chunk position"
<< seekFrameIndex << "<>" << chunkFrameIndex;
update->status = CHUNK_READ_INVALID;
Expand All @@ -88,13 +95,17 @@ void CachingReaderWorker::processChunkReadRequest(
m_pAudioSource->readSampleFramesStereo(
framesToRead, request->chunk->stereoSamples, kSamplesPerChunk);
DEBUG_ASSERT(framesRead <= framesToRead);
update->chunk->frameCount = framesRead;
update->chunk->frameCountRead = framesRead;
update->chunk->frameCountTotal = framesToRead;
if (framesRead < framesToRead) {
// Incomplete read! Corrupt file?
qWarning() << "Incomplete chunk read @" << seekFrameIndex
<< "[" << m_pAudioSource->getMinFrameIndex()
<< "," << m_pAudioSource->getFrameCount()
<< "]:" << framesRead << "<" << framesToRead;
SampleUtil::clear(
request->chunk->stereoSamples + (framesRead * kChunkChannels),
(framesToRead - framesRead) * kChunkChannels);
update->status = CHUNK_READ_PARTIAL;
} else {
update->status = CHUNK_READ_SUCCESS;
Expand Down
3 changes: 2 additions & 1 deletion src/cachingreaderworker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class AudioSourceProxy;
// sampleForChunk()
typedef struct Chunk {
int chunk_number;
int frameCount;
int frameCountRead;
int frameCountTotal;
CSAMPLE* stereoSamples;
Chunk* prev_lru;
Chunk* next_lru;
Expand Down
32 changes: 23 additions & 9 deletions src/sources/soundsourcemp3.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,18 @@ SINT SoundSourceMp3::seekSampleFrame(SINT frameIndex) {
DEBUG_ASSERT(isValidFrameIndex(m_curFrameIndex));
DEBUG_ASSERT(isValidFrameIndex(frameIndex));

// Handle trivial case
if (m_curFrameIndex == frameIndex) {
// Nothing to do
return m_curFrameIndex;
}
// Handle edge case
if (getMaxFrameIndex() <= frameIndex) {
// EOF reached
m_curFrameIndex = getMaxFrameIndex();
return m_curFrameIndex;
}

SINT seekFrameIndex = findSeekFrameIndex(
frameIndex);
DEBUG_ASSERT(SINT(m_seekFrameList.size()) > seekFrameIndex);
Expand Down Expand Up @@ -523,10 +535,14 @@ SINT SoundSourceMp3::seekSampleFrame(SINT frameIndex) {
// Decoding starts before the actual target position
DEBUG_ASSERT(m_curFrameIndex <= frameIndex);

// Skip (= decode and discard) prefetch data
const SINT skipFrameCount = frameIndex - m_curFrameIndex;
skipSampleFrames(skipFrameCount);
DEBUG_ASSERT(m_curFrameIndex == frameIndex);
// Skip (= decode and discard) all samples up to the target position
const SINT prefetchFrameCount = frameIndex - m_curFrameIndex;
const SINT skipFrameCount = skipSampleFrames(prefetchFrameCount);
DEBUG_ASSERT(skipFrameCount <= prefetchFrameCount);
if (skipFrameCount < prefetchFrameCount) {
qWarning() << "Failed to prefetch sample data while seeking"
<< skipFrameCount << "<" << prefetchFrameCount;
}

DEBUG_ASSERT(isValidFrameIndex(m_curFrameIndex));
return m_curFrameIndex;
Expand Down Expand Up @@ -580,7 +596,7 @@ SINT SoundSourceMp3::readSampleFrames(
if (isUnrecoverableError(m_madStream)) {
qWarning() << "Unrecoverable MP3 frame decoding error:"
<< mad_stream_errorstr(&m_madStream);
// Abort
// Abort decoding
break;
}
if (isRecoverableError(m_madStream)) {
Expand All @@ -597,14 +613,12 @@ SINT SoundSourceMp3::readSampleFrames(
<< mad_stream_errorstr(&m_madStream);
}
}
// Acknowledge error...
m_madStream.error = MAD_ERROR_NONE;
// ...and continue
// Continue decoding
}
}
if (pMadThisFrame == m_madStream.this_frame) {
qDebug() << "Retry decoding MP3 frame @" << m_curFrameIndex;
// Retry
// Retry decoding
continue;
}

Expand Down

0 comments on commit c390cba

Please sign in to comment.