From df2456f5592b54bf659fcdf8f2b22fb8dd72d6fd Mon Sep 17 00:00:00 2001 From: hl-valdemar Date: Sat, 10 Oct 2026 01:26:26 +0200 Subject: [PATCH] fix: preserve audio sample continuity across timestamp rounding --- Checks/audio-continuity.cpp | 96 +++++++++++++++++++++++++++++++++++++ Makefile | 23 ++++++++- README.md | 4 ++ Sources/Core/player.cpp | 10 +++- Sources/macOS/Output.mm | 8 ++++ 5 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 Checks/audio-continuity.cpp diff --git a/Checks/audio-continuity.cpp b/Checks/audio-continuity.cpp new file mode 100644 index 0000000..80b4155 --- /dev/null +++ b/Checks/audio-continuity.cpp @@ -0,0 +1,96 @@ +#include "../Sources/Core/player.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { +std::array capture{}; +size_t capturedFrames = 0; +void require(bool condition, const char *message) { + if (!condition) { std::fprintf(stderr, "FAIL audio: %s\n", message); std::exit(1); } +} +void play(const std::string &path) { + capturedFrames = 0; + std::unique_ptr player(player_create(), player_destroy); + require(player && player_open(player.get(), path.c_str()), "open fixture"); + bool started = false, ended = false; + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(12); + while (std::chrono::steady_clock::now() < deadline) { + auto state = player_get_state(player.get()); + if (state.status == PLAYER_ERROR) { + std::fprintf(stderr, "%s\n", state.error); + require(false, "playback error"); + } + if (state.status == PLAYER_PAUSED && !started) { + require(player_play(player.get()), "play fixture"); + started = true; + } + if (state.status == PLAYER_ENDED) { ended = true; break; } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + require(ended, "playback completes"); + player.reset(); // Stop/join before inspecting callback-owned capture storage. +} +size_t firstActive(const float *samples, size_t frames) { + size_t first = 0; + while (first < frames && std::abs(samples[first * 2]) < .002f) ++first; + require(first < frames, "tone is present"); + return first; +} +} + +// Linked only into the check build. No allocation, locks, or file I/O in callback. +void player_check_audio(const float *samples, size_t frames) { + size_t count = std::min(frames, capture.size() / 2 - capturedFrames); + std::memcpy(capture.data() + capturedFrames * 2, samples, count * 2 * sizeof(float)); + capturedFrames += count; +} + +int main(int argc, char **argv) { + require(argc == 2, "fixture directory required"); + std::string directory = argv[1]; + for (const char *name : {"tone441.wav", "tone480.wav", "tone441.mka", "tone-aac.m4a"}) { + std::string path = directory + "/" + name; + std::ifstream input(path + ".f32", std::ios::binary | std::ios::ate); + require(bool(input), "PCM reference exists"); + auto bytes = input.tellg(); + require(bytes > 0 && bytes % (2 * sizeof(float)) == 0, "stereo reference format"); + std::vector reference(static_cast(bytes) / sizeof(float)); + input.seekg(0); + require(bool(input.read(reinterpret_cast(reference.data()), bytes)), "read PCM reference"); + play(path); + size_t frames = reference.size() / 2; + size_t start = firstActive(capture.data(), capturedFrames); + size_t referenceStart = firstActive(reference.data(), frames); + require(start >= referenceStart, "capture alignment"); + size_t offset = start - referenceStart; + require(capturedFrames >= offset + frames, "all output samples reach callback"); + float maximumError = 0; + for (size_t i = 0; i < reference.size(); ++i) + maximumError = std::max(maximumError, std::abs(capture[offset * 2 + i] - reference[i])); + std::printf("%s: max sample error %.8f\n", name, maximumError); + require(maximumError < .00003f, "continuous callback PCM matches reference (no inserted/dropped samples)"); + } + // Continuity must not erase a real 250 ms PTS gap in the source. + play(directory + "/tone-gap.mka"); + size_t offset = firstActive(capture.data(), capturedFrames) - 1; + require(capturedFrames > offset + 48000 * 2, "gapped media is presented"); + for (size_t i = 51840; i < 57600; ++i) + require(std::abs(capture[(offset + i) * 2]) < 1e-6f, "real timestamp gap remains silent"); + double energy = 0; + for (size_t i = 62000; i < 66800; ++i) { + double value = capture[(offset + i) * 2]; + energy += value * value; + } + require(energy / 4800 > .001, "audible tone resumes after real gap"); + std::puts("PASS callback PCM continuity at 44.1/48 kHz, coarse timestamps, and real timestamp gap"); +} diff --git a/Makefile b/Makefile index 5f8ed59..15f23d1 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ FRAMEWORKS := -framework AppKit -framework AudioToolbox -framework CoreAudio -fr OBJECTS := .build/player.o .build/Output.o APP := .build/Player.app -.PHONY: all run check +.PHONY: all run check check-audio all: $(APP)/Contents/MacOS/Player $(APP)/Contents/Info.plist .build: @@ -35,10 +35,29 @@ run: all .build/playback-check: Checks/playback.mm $(OBJECTS) Makefile $(CXX) $(CXXFLAGS) -fobjc-arc $< $(OBJECTS) $(FF_LIBS) $(FRAMEWORKS) -o $@ -check: .build/playback-check +check: .build/playback-check check-audio mkdir -p .build/fixtures ffmpeg -hide_banner -loglevel error -y -f lavfi -i testsrc2=size=320x180:rate=24 -f lavfi -i sine=frequency=440:sample_rate=44100 -t 4 -c:v mpeg4 -bf 2 -c:a aac .build/fixtures/av.mp4 ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/av.mp4 -an -c:v copy .build/fixtures/video.mp4 ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/av.mp4 -vn -c:a pcm_s16le .build/fixtures/audio.wav ffmpeg -hide_banner -loglevel error -y -f lavfi -i testsrc2=size=160x90:rate=12 -t 1 -vf 'format=yuv420p10le,setparams=color_primaries=bt2020:color_trc=smpte2084:colorspace=bt2020nc' -c:v ffv1 .build/fixtures/hdr.mkv .build/playback-check .build/fixtures + +.build/Output-check.o: Sources/macOS/Output.mm Sources/Core/output.hpp Makefile | .build + $(CXX) $(CXXFLAGS) -fobjc-arc -DPLAYER_AUDIO_CHECK -c $< -o $@ + +.build/audio-continuity-check: Checks/audio-continuity.cpp .build/player.o .build/Output-check.o Makefile + $(CXX) $(CXXFLAGS) $< .build/player.o .build/Output-check.o $(FF_LIBS) $(FRAMEWORKS) -o $@ + +check-audio: .build/audio-continuity-check + mkdir -p .build/fixtures + ffmpeg -hide_banner -loglevel error -y -f lavfi -i sine=frequency=440:sample_rate=44100 -t 2 -c:a pcm_s16le .build/fixtures/tone441.wav + ffmpeg -hide_banner -loglevel error -y -f lavfi -i sine=frequency=440:sample_rate=48000 -t 2 -c:a pcm_s16le .build/fixtures/tone480.wav + ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/tone441.wav -c:a copy .build/fixtures/tone441.mka + ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/tone441.wav -c:a aac .build/fixtures/tone-aac.m4a + ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/tone441.wav -af "asetpts='PTS+gte(T,1)*0.25/TB'" -c:a pcm_s16le .build/fixtures/tone-gap.mka + ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/tone441.wav -ar 48000 -ac 2 -f f32le .build/fixtures/tone441.wav.f32 + ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/tone480.wav -ar 48000 -ac 2 -f f32le .build/fixtures/tone480.wav.f32 + ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/tone441.mka -ar 48000 -ac 2 -f f32le .build/fixtures/tone441.mka.f32 + ffmpeg -hide_banner -loglevel error -y -i .build/fixtures/tone-aac.m4a -ar 48000 -ac 2 -f f32le .build/fixtures/tone-aac.m4a.f32 + .build/audio-continuity-check .build/fixtures diff --git a/README.md b/README.md index 7e0b088..a5ed77f 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,10 @@ make check Generates short fixtures and exercises the real playback interface and native outputs: audio/video, video-only, audio-only, pause, paused/playing seek, completion, replay, close, HDR rejection, failed-open recovery, and destruction while playing. It initializes the actual Metal shaders and uses an offscreen native window. It requires a working audio device and graphical macOS session, and plays short sine-wave audio. It is a runtime smoke check, not a mocked clock/device test. +`make check-audio` runs just the audio-quality check. A check-only hook captures actual Core Audio callback PCM into preallocated storage and compares every sample against FFmpeg's reference output for PCM/AAC, 44.1/48 kHz, and coarse container timestamps. A separate gapped fixture verifies that real timestamp gaps remain silent. Release builds contain no capture hook. + +Continuous audio uses resampled sample counts for its output timeline; packet timestamp quantization and rounded resampler delay must not insert silence or drop samples between blocks. + Visual correctness, perceived A/V synchronization, and control usability still need a check in the visible app. ## Current limits diff --git a/Sources/Core/player.cpp b/Sources/Core/player.cpp index 0fd9152..c4d9b46 100644 --- a/Sources/Core/player.cpp +++ b/Sources/Core/player.cpp @@ -404,6 +404,7 @@ struct Player { } void convertAudio() { + const bool continuing = resampler != nullptr; auto &track = tracks[0]; const int rate = decoded->sample_rate > 0 ? decoded->sample_rate : track.codec->sample_rate; const AVChannelLayout *layout = decoded->ch_layout.nb_channels > 0 ? @@ -424,8 +425,13 @@ struct Player { throw std::runtime_error("Audio format changes within a track are not supported"); } const double timestamp = framePts(track, decoded); - track.nextPts = timestamp + static_cast(decoded->nb_samples) / rate; - pcmPts = timestamp - static_cast(swr_get_delay(resampler, rate)) / rate; + const double tick = av_q2d(format->streams[track.index]->time_base); + const bool contiguous = continuing && std::abs(timestamp - track.nextPts) <= tick + 1.0 / rate; + // Quantized packet PTS and rounded resampler delay must not insert/drop PCM samples. + const int64_t delayBase = static_cast(rate) * sampleRate; + pcmPts = contiguous ? resampleNextPts : + timestamp - static_cast(swr_get_delay(resampler, delayBase)) / delayBase; + track.nextPts = (contiguous ? track.nextPts : timestamp) + static_cast(decoded->nb_samples) / rate; const int capacity = swr_get_out_samples(resampler, decoded->nb_samples); check(capacity, "Sizing resampled audio"); if (capacity > audioFrameLimit) diff --git a/Sources/macOS/Output.mm b/Sources/macOS/Output.mm index 9647507..10b6a70 100644 --- a/Sources/macOS/Output.mm +++ b/Sources/macOS/Output.mm @@ -17,6 +17,11 @@ extern "C" { #include #include +#ifdef PLAYER_AUDIO_CHECK +// Check builds observe the actual callback samples; release builds have no hook. +extern void player_check_audio(const float *samples, size_t frames); +#endif + namespace { constexpr size_t capacity = 48000; double host_seconds(uint64_t ticks) { @@ -258,6 +263,9 @@ struct Output::Impl { size_t first = std::min(frames, capacity - size_t(r % capacity)); std::memcpy(destination, self.samples.data() + (r % capacity) * 2, first * 2 * sizeof(float)); std::memcpy(destination + first * 2, self.samples.data(), (frames - first) * 2 * sizeof(float)); + #ifdef PLAYER_AUDIO_CHECK + player_check_audio(destination, count); + #endif if (frames) { self.clockVersion.fetch_add(1, std::memory_order_acq_rel); self.anchorPTS.store(self.origin + double(r) / 48000, std::memory_order_relaxed);