#1399 closed defect (fixed)
BS2: Bad audio/video sync in cutscenes
| Reported by: | eriktorbjorn | Owned by: | eriktorbjorn |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Sword2 |
| Version: | Keywords: | ||
| Cc: | Game: | Broken Sword 2 |
Description
For some reason, when I run BS2 on my Linux box cutscene playback is smooth and well synced. However, when I try it on a Windows box, the playback is jerky (but well synced).
The jerkiness does not come from dropped frames or anything like that. The Windows box is much faster than my Linux box anyway. Instead, it appears to be because the getSamplesPlayed() method of finding out how much of the sound has played is simply too coarse in this case.
Each time readBuffer() is called, it requests 11024 new samples, which means the returned value from getSamplesPlayed() is increased by 5512. (When I play MI1CD each call to readBuffer() requests 512 samples. I don't know why the difference is so great.)
From what I understand, this means that the approximation for number video frames to play increases in steps of three.
I can make the playback smoother by having getSamplesPlayed() return an approximation based on when readBuffer() was last called. I haven't checked how that impacts syncing - the playback code makes assumptions about the returned value being inaccurate - but that hack is probably too ugly to live anyway...
Ticket imported from: #880484. Ticket imported from: bugs/1399.
Attachments (4)
Change History (25)
by , 21 years ago
| Attachment: | vorbis.diff added |
|---|
comment:1 by , 21 years ago
| Owner: | set to |
|---|
comment:2 by , 21 years ago
I'd better point out that the patch/hack is wrong anyway. It will not notice if the user calls read() instead of readBuffer() (then again, I thought read() was obsolete), and the approximation should subtract, not add, since the most recently added samples haven't yet been played.
It was just what I used to test my theory.
comment:3 by , 21 years ago
When I run the cutscenes on my Linux box, readBuffer() is asked for 4096 samples instead of 11024. At this rate, the jerkiness is not very noticeable.
In the MI1CD case it asked for 512, just as the Windows version did.
I have absolutely no idea what causes this difference.
comment:4 by , 21 years ago
Max, I don't really know much about the streaming code - do you have any ideas on this one?
comment:5 by , 21 years ago
| Owner: | changed from to |
|---|
comment:6 by , 21 years ago
| Owner: | removed |
|---|
comment:7 by , 21 years ago
The number of bytes passed to readBuffer essentially depends on the backend, and on the sampling rate of the data. First off, if the sampling rate differs from the backend's output rate, a rate converter is used, which fetches 512 samples at a time. This is why in one case you see 512 on both systems. If sample rate and audio rate are equal, a special "copy rate" converter is used, which reads all the data it needs at once. How much it reads depends on how much the backend requests from the mixer, i.e. this is will differe between different OSes.
However, this is IMO *not* a flaw in the mixer. Yes, in the first case you are seeing read's of 512 bytes, but they are all not actually being outputed yet - first, it'll read enough 512byte blocks to fill up the 4096 / 11024 samples requested by the backend; and only then those are passed to the backend and played back. So the higher granularity is purely an illusion.
To me this sounds as if the animation sync logic is poor; maybe it always assumes that at most one frame of audio gets played, and then doesn't cope properly with the case when multiple frames get played at once...
comment:8 by , 21 years ago
Having thought about this some more, the whole getSamplesPlayed() approach seems pretty hopeless. How about simply measuring the amount of time since the call to playInputStream() instead? That should be almost as reliable (unless the sound starts stuttering, in which case syncing is the least of our troubles), immensely more fine-grained and not as susceptible to the whims of the backend and mixer.
Of course, it will break if we ever allow pausing through cutscenes. So let's not do that, then.
I'm going to see if I can come up with a patch for the BS2 case, and someone else can fix up the BS1 case afterwards. Someone with access to the re-encoded BS1 cutscene files, preferably.
comment:9 by , 21 years ago
I haven't checked how well this actually works compared to the old one. That is, I know the playback is smoother under Windows, naturally, but I haven't really compared how well the audio is synced.
But it's at least something to experiment with.
comment:10 by , 21 years ago
This is a slightly cleaned up version of the previous patch. I've verified that it runs in both 8 and 16-bit mode. (There's usually an ugly flash in 8-bit mode when the palette changes, but perhaps there isn't much to be done about that.)
by , 21 years ago
| Attachment: | sword2-animation2.diff added |
|---|
Slightly cleaned up version of the previous patch
comment:11 by , 21 years ago
| Component: | → Engine: Sword2 |
|---|---|
| Game: | → Broken Sword 2 |
comment:12 by , 21 years ago
I had to update the patch anyway to deal with some whitespace changes, so I figured I might as well update the BS1 animation code while I was at it.
The BS1 code will need some more work, but that's true regardless of whether or not this patch is accepted.
comment:13 by , 21 years ago
| Owner: | set to |
|---|
comment:14 by , 21 years ago
Assigning to Ender again, since the proposed patch doesn't use the sound code for syncing. :-)
comment:15 by , 21 years ago
Fingolfin suggested adding a AudioStream::getTimeElapsed() function that would return basically the same information as getSamplesPlayed(), but with interpolation to make it linear.
Which means we're getting close to my original hack again. :-)
In other words, don't apply the current patch right now.
comment:16 by , 21 years ago
The problem with this is that the audio device doesn't have to use the same clock as the system. This wont hurt in small animations as the audio time and the system time usually differ by only a small amount but in the long intro I get a lager difference. The difference is quite visible in the intro, when George knocks on the door and later when he is knocked down.
I know that the current situation is not nice. Video players use a feature provided by the sound driver that returns the exact number of samples sent to the sound card, which is normally exact enough.
But as we don't have this, I'd suggest we add some guesswork to the getSamplesPlayed() function. Something like time since last read call times samplerate
comment:17 by , 21 years ago
I've submitted a patch that adds a getChannelElapsedTime() function to the mixer that returns an approximation of how long a sound has been playing. It's not perfect by any means, and I don't know if it'll be accepted anyway, but if it is it should be trivial to change the cutscenes to use that.
I've only tried it on my Linux box though, where even the current method works well.
comment:18 by , 21 years ago
I've tried it on the Windows box that has trouble with the current version. Seems to work fine.
comment:19 by , 21 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:20 by , 21 years ago
I've applied the patch, and updated the BS2 cutscene code. (I'm working on the BS1 code now.) Closing.
comment:21 by , 21 years ago
| Owner: | changed from to |
|---|
Ugly hack against a January 20 CVS snapshot