Increase the maximum video buffer size. #1351

wants to merge 2 commits into


None yet
4 participants

zewt commented Sep 1, 2012

Fixes (see ticket for test file).

The video stream was filling up while the audio stream still needed data.

I haven't been able to test this for a lot of files, since the master branch is pretty broken right now (falls over after playing one or two files), but it fixes the issue for every file I've tried. It may be possible to increase it by a smaller amount.

(The more robust fix is probably to always buffer more data in either the audio or video stream if the other stream still needs data, instead of using a hard limit and hoping it's enough.)


elupus commented Sep 2, 2012

This really just feel like a broken file. 12 seconds desync between audio
and video is just wrong. And that really should not even be a problem when
played back. It should issue seeks in the file to get the audio and video
in the correct order. It feel like bug further down.


zewt commented Sep 2, 2012

The files work in every other player, so in practice it's not the file that's broken. (If it works in most players, then people are going to continue--no matter how much we yell at them, if we can even find them--to encode files in this way.) Also, this isn't an isolated issue; I've hit this many times (or else I wouldn't be bothered enough to try to fix it), and XBMC is the only player that has trouble with it.

Seeking around the file would be wrong, because that would fall apart on media with expensive seeks (eg. DVDs, internet streaming). That would also require a major overhaul of CDVDPlayer, from what I can tell.


elupus commented Sep 2, 2012

Ffmpeg seek internally on mp4s.


FernetMenta commented Sep 2, 2012

I remember a problem with a huge audio offset on a iptv channel. Might be the same issue. I don't think they violated the spec of rts streams (haven't had a chance to look into the spec myself). The recording played fine in VLC.


elupus commented Sep 2, 2012

They very likely did. Live stream should not require player to buffer
several seconds of video data to play in sync with audio. The other way
around may be more okey, but it's still the same thing. Audio and video
should be received synced.

For mp4's it's different. They contain an index that says where each sample
is in the stream. The demuxer then uses this index to jump to each sample
in decode time turn, and output it. To allow streaming, we allow up to
8seconds skew between samples if they are in byte order. May there is an
issue with that, so we allow too large skew in samples in libavformat.
(demuxer is in libavformat/mov.c).

Or maybe we fail to parse the mp4 index, so we just play them in byte
order. Have to slow internet access to check that file right now.


fritsch commented Sep 2, 2012

Here is such a stream, dumped via alice iptv:

I kept it around, so cannot get new dumps - as i changed cable provider. The patch "makes the guy dancing more nicely" - so it hugely improves playback quality.


zewt commented Sep 2, 2012

Going back to my original thought: it actually seems fairly straightforward for CDVDPlayer to continue buffering if either stream needs more data.

-    if ((!m_dvdPlayerAudio.AcceptsData() && >= 0)
-    ||  (!m_dvdPlayerVideo.AcceptsData() && >= 0))
+    if (( < 0 || !m_dvdPlayerAudio.AcceptsData())
+    &&  ( < 0 || !m_dvdPlayerVideo.AcceptsData()))

This way, more data is read until both players' queues are full. This effectively makes the values into desired buffer sizes, rather than maximums. After doing this, I can reduce both audio and video buffering significantly without causing playback issues. (And they would want to be reduced; they're currently tuned as pessimistic maximums, not desired values.) The amount buffering simply grows beyond that value if required by the file.

Of course, this also means that there's no hard limit on buffering. If that actually happened--if it tried to buffer a huge amount of data--then the file isn't going to work anyway, so it should just show an error and stop if either queue becomes unreasonably large.

(I'm sure there are other issues with this; for example, I don't know the logic of the "GetLevel() > 50" cases in CheckStartCaching, which this would affect.)

@elupus elupus closed this Oct 25, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment