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

[Reimar Döffinger]

On Sat, Apr 30, 2011 at 03:26:12PM +0200, David Kuehling wrote:
> >>>>> "Reimar" == Reimar Döffinger <Reimar.Doeffinger at gmx.de> writes:
> > On Sat, Apr 30, 2011 at 03:06:38PM +0200, David Kuehling wrote:
> >> Hmm, well, AFAIK, for the Theora embedding in Ogg as defined by the
> >> Theora spec, frametime is a constant.  Variable framerate is only
> >> possible by using a short frametime, and injecting emtpy packets
> >> every now and then to pad between actual frames.
> >> 
> >> So that's what I was referring to.  Did I overlook something?
> 
> > At the very least that theora might be stored in other containers like
> > mkv without such arbitrary restrictions.  
> 
> This is why I wrote _Ogg_/Theora is constant-framerate.  You're right my
> patch checks for theora but should also check for the ogg container.

I was wondering if it was intentional since the check for Ogg is
after all right below.

> > Then there's the thing that I am unsure that e.g. the FFmpeg muxer
> > does behave like this.
> 
> You really mean muxer not demuxer?  

Yes.

> > Not sure how "short frametime" is defined, but I would take a guess
> > that this might be broken by your change.  
> 
> "Short frametime" is not defined.  Using a short frametime is just a
> hack to emulate variable-framerate video with a fixed-framerate video
> format such as Ogg/Theora.  No without my change that wouldn't work at
> all, because all padding frames that come as 0-sized packets would get a
> frametime of 0 assigned, thus the video plays back at 1/short_frametime
> instead of the actual variable framerate of the video.

I must be something missing in all this, because the frame time calculation
you disable is based on pts, so in theory the delays should at most shift
to the wrong place.
Which is where my other theory comes from, that what really does wrong is
that d_video->pts is updated incorrectly.
Another possibility would be to change the buffer_pos condition to also
use the next packet pts when the buffer size is 0.
Currently I am unsure which solution makes most sense, but yours unfortunately
right now is the one I see most potential issues with (also think of other
video formats like VP8 in Ogg etc.).