[MPlayer-dev-eng] [BUG] [PATCH] Ogg/Theora frametime broken on 0-length packets

[Reimar Döffinger]

On Sat, Apr 30, 2011 at 03:21:19PM +0200, David Kuehling wrote:
> >>>>> "Reimar" == Reimar Döffinger <Reimar.Doeffinger at gmx.de> writes:
> > On Sat, Apr 30, 2011 at 02:34:20PM +0200, David Kuehling wrote:
> >> I noticed that a/v sync drifts in theora files that contain 0-length
> >> video demux packets (i.e. frames where the image doesn't change at
> >> all), when played back with -demuxer ogg.  The libavformat demuxer
> >> seems to work correctly.  However it uses so much memory (a leak),
> >> that it won't work on the target platform (NanoNote: 32MB RAM, no
> >> swap).
> 
> > Please provide a better bug report so we can fix this, -demuxer ogg is
> > unlikely to work forever.
> 
> Sorry I don't understand that sentence.  You mean demuxer ogg is
> deprecated?

Yes.

> WRT 'better bug report': what else (apart from a test-case
> video) do you need?

For example an explanation why you call it a "leak" (did you use
e.g. valgrind?), if/how sure you are it is the demuxer and not
the decoder (as said I think a switch of demuxers implies a
switch of decoders).

> >> Looking closer, I found that ds_get_next_pts() returns the *current*
> >> pts when the packet read last has zero length.  This is due to the
> >> check for 'ds->buffer_pos' to see whether data have already been read
> >> from the *current* packet.  Of course, ds->buffer_pos will stay when
> >> a zero-length packet was read, so that fails.
> 
> > There are multiple possible solutions, the most correct _probably_ to
> > stop the Ogg demuxer from outputting 0-size packets.  
> 
> This would only work if we would then increase the frametime of the
> previous video frame accordingly.  I.e. before we enqueue a new video
> packet we'd first have to go on reading any consecutive 0-size packets
> (which are AFAIK 5 byte when counting the headers), to determine
> frametime.  Sure that adding this kind of complexity to the ogg demuxer
> is the right solution?

Just not calling ds_add_packet for the 0-size frames should actually
take care of all that, so it should be about the same amount of code/
complexity as your patch.
However it might break things for codecs where a 0-size packet has
a special meaning.

> > Though this also might be an issue of the decoder behaving
> > incorrectly.  Are you sure it is not related to that you end up using
> > libtheora when using -demuxer ogg while using fftheora with -demuxer
> > lavf?  
> 
> I guess the decoder does not play a role, because vd_theora.c just
> returns NULL (i.e. skip frame) when receiving a 0-sized packet.

Well, I do think that is correct, but not that there is also
the mpi_no_picture that e.g. vd_ffmpeg.c can return.
That can't really cause the issue but there might be other differences.