[MPlayer-dev-eng] [PATCH] further dvr-ms playback improvements

[Uoti Urpala]

On Fri, 2006-08-25 at 23:33 +0200, Nico Sabbi wrote:
> the duration reported is correct, but obviously the file doesn't play 
> correctly
> (from a visual point of view)

The timestamps are uneven, but I think it's not "accelerated" if the
average speed is correct.

> >Apparently several demuxers use that to
> >indicate that the video is VFR and does not have a constant frame rate.
> >It's interpreted by video.c; if fps is set to some other value then that
> >is used as frame duration ignoring timestamps, fps=1000 means packet
> >timestamps are used (the code depends on demuxer type, does affect
> >DEMUXER_TYPE_ASF). 
>
> the interpolation used is hopelessly broken (because of b-frames)

If that code is used (without -correct-pts) it assumes that the
timestamps have been reordered by the demuxer.

> >>but mplayer from svn doesn't even init when sh_video->pts == 0,
> >
> >That means the value wasn't inited at all.
>
> here I meant that "mplayer from svn doesn't even init when sh_video->fps 
> == 0"

Yes I understood that; I meant the fps value has not been inited (as
MPlayer expects it to be inited to either a constant FPS or to 1000 for
VFR).

> >>thus I had to patch one of the decoders to assign the video framerate
> >
> >Why? What was the point? Why did you want to remove the fps=1000
> >assignment? Sure that is an ugly way to handle it but does it have
> >anything to do with the problems with that file? 
>
> not particularly with that file; the problem is that the fps 
> interpolation in video.c is really broken

Well it assumes reordered timestamps and has incorrect lag with B
frames. I don't see how your patch would help with that though, and
-correct-pts is there to avoid using that code anyway.

> >Do you believe that the
> >decoder has more accurate timestamp information which cannot be obtained
> >in the demuxer 
> >
> absolutely not.
> 
> It seems I can't explain myself very well today, so let me retry :)
> 
> 1) The assignment sh->fps = 1000 is _arbitrary_ and completely fake, 
> thus it should be avoided.

It's an arbitrarily selected value, but one that has been selected to
have a special meaning in a couple of places (VFR). Given that other
code, the code assigning fps=1000 in the asf demuxer is NOT arbitrary.

> 2) If we avoid that assignment sh->fps will be initialized with 0, 
> leading mplayer to
> refuse to play the file.

Yes, currently MPlayer treats that as "accidentally uninitialized". If
you don't want to set constant FPS you should set 1000 (from the current
demuxer point of view; 1000 isn't special in that many places so the
behavior wouldn't be hard to change, but anyway the current value of
1000 shouldn't break things).

> 3) In order to play the file sh->fps will have to be initialized  to a 
> sensible value;

Where 1000 is currently a "sensible value" meaning "use the timestamps
of individual packets".

> since the demuxer doesn't know this value the only other component that can
> possibily set it is the video decoder

The demuxer can just set it to 1000.

> 4) from now on the _initial_  value of sh->fps is set and everything 
> will proceed as usual,
> so the value of sh->pts will depend on the demux_packets pushed to 
> demuxer->video
> by the demuxer (hence sh->pts will be correct irrispective of sh->fps)

No. If you set fps to a value above 1 and not 1000 and use the video.c
code, then that will be used for frame durations INSTEAD of the
timestamp deltas between packets from the demuxer.


Btw the formatting of your posts seems broken, as if lines had first
been wrapped to one length and then each line was individually split
into two at a shorter limit.