Repository navigation
fix(avplayer): correct audio-video playback timing and sync - #5995
Conversation
982c760 to
8b06c36
Compare
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
8b06c36 to
83e7d26
Compare
jotitadev
left a comment
There was a problem hiding this comment.
Linux run: head 83e7d26e0 merged onto main 44df21632 (clean merge). The description says the full build and suite were not run, so here they are.
Machine: Ubuntu 26.04, Intel i7-8700, GCC 15.2, full build, non-GPU tests (Release, BUILD_TESTING=ON).
- Build: full build passes.
- AudioOut/AvPlayer tests with the PR: all 18
audio_out*tests andguest_avplayerpass,audio_out_exincluded (the description mentions it terminating on your side). - Tests without the fix (this PR's two test files on
main'sSceTypes.hpp,AudioOut.cppand AvPlayer sources):audio_out_last_output_timeaborts: "Independent port output waited 910 ms", "pacing one audio port must not block output on another port".guest_avplayerfails: "PS5 audio stream must use type 2 so Unity enables it". It stops at the first check, so the frame-rate checks at 0x40/0x48 were not reached without the fix.audio_out_expasses either way.
- Non-GPU suite (
ctest -E "^agc"): no regressions; the only failures arefile_writer_binaryandfile_writer_text, which fail onmainon this machine too.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Independent Linux check of The test-only commit exercises real public AudioOut calls with an interposed SDL queue and fixed guest clock, holding port A in backpressure until explicitly released. On main, output and close on independent port B fail to complete before A is released; the same-port close control passes. With this PR, all three pass. This supplements the virtual-port tests with controlled device-queue coverage. Removing only the same-port close lock makes the control fail on access to the reset device; restoring it passes. The fixture executable is byte-identical across these latter runs (SHA-256 Separate no-sync contract concern: I made a separate diagnostic probe: read one audio frame to activate the audio-backed clock, then request decoded video without consuming further audio and measure video lead over Full review source including the probe retains your original commit and intentionally exposes that probe failure. For comparison, the isolated AudioOut portion preserves your authorship, adds both test fixtures, and leaves main's AvPlayer behavior intact: full |
83e7d26 to
9e104e2
Compare
9e104e2 to
c6b9c04
Compare
|
The no-sync concern is valid. I’ve restored |
jotitadev
left a comment
There was a problem hiding this comment.
Linux run of the new head: c6b9c0498 merged onto main 5d9dcc42d (clean merge), in one tree with seven other library PRs that share no files with this one. The description says the three Linux-only SDL-interposition cases in AudioOutPortLocks.cpp were not run locally, so here they are.
Machine: Ubuntu 26.04, Intel i7-8700, GCC 15.2, full build, Release, BUILD_TESTING=ON.
- With the PR:
audio_out_independent_output,audio_out_independent_closeandaudio_out_same_port_closepass, as doguest_avplayerand the newguest_avplayer_no_sync_lead. All 20audio_out*tests plus both AvPlayer tests pass 5/5 with--repeat until-fail:5. - Without the fix (the PR's tests, but
SceTypes.hpp,AudioOut.cppand the AvPlayer sources frommain):audio_out_independent_outputandaudio_out_independent_closeabort with "AudioOut port locks: port output/close lock scope is wrong".audio_out_same_port_closepasses, as expected for the control case.audio_out_last_output_timeaborts: "Independent port output waited 909 ms".guest_avplayerandguest_avplayer_no_sync_leadboth fail at the first check, "PS5 audio stream must use type 2 so Unity enables it". So without the fix,--no-sync-lead-onlydoes not get as far as the lead measurement.
- Non-GPU suite (
ctest -E "^agc|gpu|vulkan|video_out"): no regressions. The only failures arefile_writer_binaryandfile_writer_text, which also fail onmainon this machine.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
c6b9c04 to
cdfaf79
Compare
|
Checked on Linux (merged onto current Added on rebase:
Note: the PR description no longer matches the diff (it covers sync/loop changes already merged via #5846). #6922 overlaps the per-port locking and will need a rebase after this lands. Merging once CI is green. |
cdfaf79 to
742aeb4
Compare
What
Fix audio-video playback timing and synchronization:
Tested
Windows, Evil Inside (PPSA02508): both cutscenes play with synchronized audio and video, without the premature pause before scene transitions, afterward due to an unrelated or currently unidentified issue.
Tested also My Friend Peppa Pig (PPSA03706) with initial cutscene now playing well.
Full build and complete test suite have not been run. The AudioOut Ex test terminated while handling an expected exception; that verification remains incomplete.
Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository