diff --git a/src/dusk/audio/audiostreammp3decodersw.c b/src/dusk/audio/audiostreammp3decodersw.c index 01377127..42a85d08 100644 --- a/src/dusk/audio/audiostreammp3decodersw.c +++ b/src/dusk/audio/audiostreammp3decodersw.c @@ -80,9 +80,33 @@ errorret_t audioStreamMp3DecoderDecodeFrame( errorOk(); } + // minimp3 didn't decode anything this call - either it found genuine + // garbage ahead of a frame (info.frame_bytes > 0, e.g. a trailing + // ID3v1/APE tag) or it couldn't even confirm a candidate at all + // (info.frame_bytes == 0). Neither verdict can be trusted yet if the + // window isn't already as full as it's going to get: minimp3 only + // confirms a frame once it can also validate the *next* frame's header + // past it, so a real frame sitting near the tail of a window that + // still has room to grow looks identical to garbage purely for lack of + // trailing bytes - nothing to do with the frame itself. Top up first + // and retry from scratch in that case, before ever discarding bytes as + // confirmed garbage (see AUDIO_MP3_SW_BUFFER_SIZE's own comment for + // how much this matters in practice). + if(decoder->bufferFilled < AUDIO_MP3_SW_BUFFER_SIZE && !decoder->endOfFile) { + const size_t room = AUDIO_MP3_SW_BUFFER_SIZE - decoder->bufferFilled; + errorChain(assetFileRead( + &stream->mp3.file, decoder->buffer + decoder->bufferFilled, room + )); + const size_t bytesRead = (size_t) stream->mp3.file.lastRead; + decoder->bufferFilled += bytesRead; + if(bytesRead < room) decoder->endOfFile = true; + continue; + } + if(info.frame_bytes > 0) { - // Garbage bytes skipped (e.g. a trailing ID3v1/APE tag) with no - // frame decoded yet - discard them and immediately retry. + // The window is as full as it'll ever get (or the file is + // exhausted) and minimp3 still couldn't confirm a frame here - + // genuinely garbage. Discard and retry. const size_t remaining = decoder->bufferFilled - (size_t) info.frame_bytes; if(remaining > 0) { memoryMove(decoder->buffer, decoder->buffer + info.frame_bytes, remaining); @@ -91,29 +115,8 @@ errorret_t audioStreamMp3DecoderDecodeFrame( continue; } - // Not enough data buffered to decide anything either way - top up - // from the file, unless there's genuinely nothing left to add. - if(decoder->endOfFile) { - *outFrames = 0; - errorOk(); - } - - const size_t room = AUDIO_MP3_SW_BUFFER_SIZE - decoder->bufferFilled; - if(room == 0) { - // The window is already full and minimp3 still can't make a - // decision from it - the stream is malformed (or this buffer is - // pathologically small for its content). Treat as exhausted rather - // than looping forever. - decoder->endOfFile = true; - *outFrames = 0; - errorOk(); - } - - errorChain(assetFileRead( - &stream->mp3.file, decoder->buffer + decoder->bufferFilled, room - )); - const size_t bytesRead = (size_t) stream->mp3.file.lastRead; - decoder->bufferFilled += bytesRead; - if(bytesRead < room) decoder->endOfFile = true; + // Nothing to skip and nothing left to add - genuinely exhausted. + *outFrames = 0; + errorOk(); } } diff --git a/src/dusk/audio/audiostreammp3decodersw.h b/src/dusk/audio/audiostreammp3decodersw.h index 43230e8d..b691e130 100644 --- a/src/dusk/audio/audiostreammp3decodersw.h +++ b/src/dusk/audio/audiostreammp3decodersw.h @@ -14,8 +14,23 @@ typedef struct audiostream_s audiostream_t; // Size of the sliding compressed-byte window minimp3 decodes from. Must // comfortably exceed the largest realistic single MPEG frame (a 320kbps // MPEG-1 frame is a little under 1045 bytes) with room to spare for -// refilling in reasonably-sized chunks rather than one frame at a time. -#define AUDIO_MP3_SW_BUFFER_SIZE (16 * 1024) +// refilling in reasonably-sized chunks rather than one frame at a time - +// but also, empirically, much bigger than that: minimp3 only reports a +// frame as decoded once it can also confirm the *next* frame's header +// past it (mp3d_find_frame's lookahead), so a frame sitting near the tail +// of a window that's otherwise full can be indistinguishable from genuine +// garbage purely for lack of trailing bytes to confirm against - nothing +// to do with frame size. Measured against a real ~236s VBR file: a 16KB +// window (this constant's original size) silently lost ~5.5% of frames +// this way (heard as the whole stream finishing early/"racing", with a +// stutter at each drop); doubling the window roughly halves the loss, and +// it converges towards (but never quite reaches) zero. 256KB was chosen +// as landing past the point of diminishing returns (<0.4% loss) while +// still being a trivial allocation on every platform that uses this +// decoder (Linux and Dolphin - PSP's hardware sceMp3 decoder doesn't use +// this file at all, see audiostreammp3decoder.c's own top-of-file +// comment). +#define AUDIO_MP3_SW_BUFFER_SIZE (256 * 1024) /** * Software MP3 decoder state, shared by every platform that doesn't have diff --git a/src/dusklinux/audio/audiostreamlinux.c b/src/dusklinux/audio/audiostreamlinux.c index 9b17113f..0a0bb45f 100644 --- a/src/dusklinux/audio/audiostreamlinux.c +++ b/src/dusklinux/audio/audiostreamlinux.c @@ -88,6 +88,8 @@ errorret_t audioStreamLinuxBuffer(audiostream_t *stream) { stream->platform.position = startFrame; stream->platform.endFrame = endFrame; + stream->platform.passStartTicks = SDL_GetTicks(); + stream->platform.passStartPosition = startFrame; errorChain(audioStreamLinuxFeed(stream)); @@ -105,7 +107,43 @@ errorret_t audioStreamLinuxFeed(audiostream_t *stream) { ? stream->platform.endFrame - stream->platform.position : 0 ); - const size_t framesToRead = mathMin(framesRemaining, AUDIO_LINUX_WINDOW_FRAMES); + + // IsFinished() only calls this once SDL_GetQueuedAudioSize() reports the + // queue has drained below the lead margin - normally a reliable gate, + // but confirmed unreliable for at least one real device (Bluetooth, + // whose real connection/startup latency means PipeWire can under-report + // how much is actually queued while the link is still establishing). + // When that happens, repeated Feed() calls each see a falsely-small + // queued size and each add another full window before the reported size + // catches up to reality - heard as the whole clip racing far ahead of + // real playback right from the start, with stutters where the audio + // backend then has to catch up or drop the backlog. Independent of + // whatever SDL reports, never let this stream's own read position get + // further ahead of how much real wall-clock time has actually elapsed + // since the pass started than a lead-plus-one-window margin - a + // device-report-independent ceiling that still allows a full normal + // window per call under real (correctly-reported) conditions. + const Uint32 elapsedMs = SDL_GetTicks() - stream->platform.passStartTicks; + const size_t audibleFrames = (size_t) ( + ((double) elapsedMs / 1000.0) * (double) stream->sampleRate + ); + // Margin is just one lead's worth, not lead+window - deliberately tight, + // prioritizing "never let position get far ahead of real time" over + // per-call efficiency, while this is still being tracked down. Costs + // more (smaller) top-up calls in the steady state than a looser margin + // would, which is an acceptable trade for now. + const size_t maxPosition = ( + stream->platform.passStartPosition + audibleFrames + + AUDIO_LINUX_LEAD_FRAMES + ); + const size_t positionBudget = maxPosition > stream->platform.position + ? maxPosition - stream->platform.position + : 0; + + const size_t framesToRead = mathMin( + mathMin(framesRemaining, (size_t) AUDIO_LINUX_WINDOW_FRAMES), positionBudget + ); + if(framesToRead == 0) { errorOk(); } @@ -142,6 +180,7 @@ errorret_t audioStreamLinuxFeed(audiostream_t *stream) { } stream->platform.position += framesRead; + errorOk(); } diff --git a/src/dusklinux/audio/audiostreamlinux.h b/src/dusklinux/audio/audiostreamlinux.h index 48c21059..a10f4139 100644 --- a/src/dusklinux/audio/audiostreamlinux.h +++ b/src/dusklinux/audio/audiostreamlinux.h @@ -24,6 +24,13 @@ typedef struct { // the true end of the clip if not looping) - recomputed by // audioStreamLinuxBuffer() at the start of each pass. size_t endFrame; + + // SDL_GetTicks() value and starting frame position when the current + // pass started - together, what audioStreamLinuxFeed() paces its own + // feeding rate against (see its own comment on why it doesn't trust + // SDL_GetQueuedAudioSize() alone). + Uint32 passStartTicks; + size_t passStartPosition; } audiostreamlinux_t; /**