Closed madebr closed 5 months ago
@dethrace-labs Can you please review and/or take over? Thanks.
Sure, thanks for bringing it up to latest main
!
I still experience stuttering with this. I started to experiment by trying to feed the audio 1 frame in front of the video so we always have the next audio page to consume before we run out, but its not quite working yet
Hello @dethrace-labs
Congratulations for this impressive work. I rewrote your smackw32 files to add sound playback support to videos using the supplied miniaudio backend. I started with @madebr's code, figured out the problem (sort of) with sound cracklings/stuttering, and moved it to smackw32 where I think it belongs.
There were three problems. One of them was that your frame duration measurement was incorrect. The time measurement shall be done exactly once per loop, not twice (begin / end). Because of this little mistake, the video playback speed wasn't exactly on par with the audio consumption rate, leading to frequent sound buffer starvation, causing arbitrary seek/restarts producing audible pops. The second problem, as you guessed, was that more than one audio page needs to be precached in miniaudio's linked list to avoid sound starvation at the end of the first page. The third problem, once the others were solved, appeared in so that even playing the videos with audio without any stuttering and both audio and video playing at their designated speeds, a time shift could be observed between audio and video that turned out to be simply proportional to the video length. It was very noticeable in the intro video (the longest), when the roadsign shortly spins at the end. Around 25 pages of audio needed to be precached (and the sound playback start delayed by this amount) for audio and video to become accurately synced. On the other hand, on shorter videos only 1 or 2 audio pages needed to be precached to achieve good sync. It's as if the player code was supposed to precache a certain amount of audio pages that's directly proportional to the length of the video. By trial and error I figured out that the value (total number of frames / 100) would give the exact number of pages to precache for each video. Don't ask me why it works this way... perhaps it's by design ?
As I have no write access on this repo, here are the files if you want to review them. The only files changed are smackw32.c and smackw32.h. There are a few comments and I tried to adopt your coding style. I hope you find this contribution useful: https://www.pmbaty.com/smackw32.zip
Feel free to create a new PR with your code. I will happily close this PR.
Or you can create a PR on my fork to the cutscenes-with-audio
branch that I can merge into this pr.
Thanks @PierreMarieBaty for your analysis and code changes! As @madebr said, are you able to send the changes as a pull request?
I’m sorry, I don’t use git - except when I’m forced to. My SCM of choice is SVN (don’t feed the troll) :-)
I copied smackw32.c
from the zip archive, but I still hear the audio too soon.
Interesting. Which SMK file did you test, what's your system? I tested on Win7/x64 and macOS with a French version of the Carmageddon CD. The results looked good to me. How noticeable is it? I'd like to be able to reproduce the problem.
But as I said, this precache delay was mostly guessed out and discovered by trial and error. It is certainly possible that a better mechanism exists to trigger audio and video start at the right time, that's yet to be discovered.
I run a x64 Fedora 38 Linux system.
The video I'm seeing time synchronization issues with is GARAGE1.SMK
.
That is the video that plays when starting a new game.
The video features a driver pressing some buttons, giving some synchronization references.
When I compare the video played through dethrace and through minismackerplay, the first one is not synchronized.
Okay. I compiled your minismackerplay (should have started with that), saw that it does no caching and optional conversion, and could see that all videos seem to play correctly with it. Which means the minismackerplay code must do it right.
So I added that optional check for whether resampling is needed in smackw32.c, disabled soundpage caching, and that seemed to work just as well : no sound pops, no sound starve (except when e.g. I resize the game window and the SDL loop exceptionally exceeds the frame duration - an arbitrary sync is still needed in this case. BTW minismackerplay has the same problem). Anyway, this tends to indicate that sample conversion in miniaudio from and to the same sample rates (?) produce output that's shorter than the input.
It didn't fix the sound delay in the intro video MIX_INTR.SMK though, so I checked further. I saw that dethrace uses SDL_GetTicks() and minismackerplay uses SDL_GetPerformanceCounter(). Having minismackerplay use SDL_GetTicks() too didn't help, so I made them both display the sound page sizes, and I could spot the discrepancy.
minismackerplay (right):
frame #0 sound converter framesIn 23154 framesOut 46308 frame #1 sound converter framesIn 1102 framesOut 2204 frame #2 sound converter framesIn 1102 framesOut 2204 frame #3 sound converter framesIn 1102 framesOut 2204 frame #4 sound converter framesIn 1102 framesOut 2204 ...
dethrace (wrong):
frame #0 sound converter framesIn 1102 framesOut 2204 frame #1 sound converter framesIn 1102 framesOut 2204 frame #2 sound converter framesIn 1102 framesOut 2204 frame #3 sound converter framesIn 1102 framesOut 2204 frame #4 sound converter framesIn 1102 framesOut 2204 ...
Which seemed to prove 2 things:
When things are done right, the first audio page is much bigger than the other ones, and thus lasts much longer.
The sound "precaching" effect is simply necessary because dethrace skips that frame and directly proceeds to audio frame #1 as if it was frame #0.
After some more investigation, the reason for this turned out because smk_enable_audio() was called after smk_first() in dethrace, whereas in libsmackerplay smk_get_audio() is called before it.
By reordering things in SmackOpen() I could solve the problem. There was no need to precache anything after all. As a sidenote, having a big first audio page sounds like a sensible conscious choice by the RAD SMK encoder to ensure the sound won't starve at video init on low-end machines.
I updated my smackw32 files (.c and .h). Same URL: https://www.pmbaty.com/smackw32.zip
Let me know if it sounds good this time.
Thank you! I love it when a bunch of complicated work goes in and the end result is a simple small change :). I'll make a PR with the changes.
@PierreMarieBaty I can't access the zip file - could you check your website, or share the zip somewhere else?
Weird, I can still download it. This is the zip from https://github.com/dethrace-labs/dethrace/pull/288#issuecomment-1867854056: https://dropmeafile.com/#e6f48d3f01
My server is located in Russia, that could be the reason. Your ISP might have a firewall or something blocking russian hosts maybe?
Message ID: @.***>
Closing this and will reopen shortly
tools/minismackerplay.c
using libsmacker, miniaudio and SDL2. This shows the overhead/timing/sleeps/... of dethrace should be improved