[FFmpeg-devel] About guess_correct_pts / AVFrame.best_effort_timestamp

[Alexander Strange]

On Feb 18, 2011, at 2:13 AM, Luca Barbato wrote:

> On 02/18/2011 05:53 AM, Alexander Strange wrote:
>> What side effects?
> 
> broken playing and transcoding on certain h264 in mpegts is one of them
> I noticed recently ^^;

Is that the other thread? I'd need the file to comment about anything, ts is too weird.
In valid files I guess ffplay -drp -1 (the default guessing) should work the same as -drp 1 (just pts) on ts.

>> I saw nothing in this thread except questions about how libavformat works,
>> all concrete proposals for client code were wrong.
> 
> Why?

- "Check .pts, fall back to FPS if that fails" (the only thing I saw posted in this thread) doesn't work on VFR files missing reordering info (= only DTS).
That's AVI, OGM, Xvid anywhere, ffmpeg muxing MP4 from .h264 (because the parser doesn't work), maybe some more.

- "Check pts, fall back to dts if unset" (what I just posted) doesn't work because some files set both PTS/DTS but PTS turns out to be wrong.

Examples, with attached intrusive av_logging patch, and ffplay:

http://samples.mplayerhq.hu/ogg/mpeg-in-ogm.ogm

<< pts 2 dts 2 duration 0
<< pts 3 dts 3 duration 0
<< pts 4 dts 4 duration 0

guessed 2 pts 2 dts 2
guessed 3 pts 3 dts 3
guessed 4 pts 4 dts 4
guessed 5 pts 2 dts 5 <-
guessed 6 pts 6 dts 6
guessed 7 pts 7 dts 7
guessed 8 pts 5 dts 8 <-
...
guessed 47 pts 47 dts -1 <- missing dts

http://astrange.ithinksw.net/ffmpeg/camp_wrong_dts.mp4

<< pts 0 dts 0 duration 2
<< pts 2 dts 2 duration 2
<< pts 4 dts 4 duration 2

guessed 0 pts 0 dts 2
guessed 4 pts 4 dts 4
guessed 6 pts 6 dts 6
guessed 8 pts 2 dts 8 <-
guessed 10 pts 10 dts 10
guessed 12 pts 12 dts 12
guessed 14 pts 8 dts 14 <-
guessed 16 pts 16 dts 16
guessed 18 pts 18 dts 18
guessed 20 pts 14 dts 20 <-
...
guessed 58 pts 56 dts 58
guessed 58 pts 58 dts -1 <- missing dts, non-monotone time

This could sort-of be fixed in libavformat by only setting dts in this case. But then we can't assign times to frames flushed at the end (up to 16 in h264), because we don't have dts for them. Not sure what to do here.

I don't have any examples of wrong pts that aren't in files missing it entirely.

After that we have files with out of order DTS. I don't know any files where this happens, only some where PTS/DTS are equally wrong. Also I have no idea what the code was added for, because no samples are mentioned in the svn commit.

Example:

http://samples.mplayerhq.hu/archive/all/mpeg%2bmpeg2video%2bac3%2b%2bnon_monotone_timestamps.mpg

<< pts -1 dts 7997 duration 3600
<< pts 18797 dts 11597 duration 3600
<< pts 15197 dts 15197 duration 3600
<< pts 18797 dts 18797 duration 3600
<< pts -1 dts 18797 duration 3600

>> guessed 11597 pts -1 dts 11597 <-
>> guessed 15197 pts 15197 dts 15197
>> guessed 18797 pts 18797 dts 18797
>> guessed 18797 pts 18797 dts 18797 <- both wrong, guess is unhelpful

The code is useful for counting the faults, but has no effect for playback. It could be removed.

Also, with nofillin:

<< pts 7997 dts 7997 duration 0
<< pts 18797 dts 11597 duration 0
<< pts -1 dts -1 duration 0
<< pts -1 dts -1 duration 0
<< pts -1 dts -1 duration 0

>> guessed 7997 pts 7997 dts 11597
>> guessed -1 pts -1 dts -1
>> guessed -1 pts -1 dts -1
>> guessed 18797 pts 18797 dts -1

No duplicate times appear. Maybe the duplicate frame "<< pts -1 dts 18797" with fillin is a lavf bug? I don't know enough to say about this file.

>> I think more valid times _could_ be made to work in libavformat, with better
>> AVParsers that don't quite exist, but that would leave something like this:
>> avcodec_decode_video(..)
>> pts = avctx->pkt_pts ? avctx->pkt_pts : avctx->pkt_dts;
> 
>> Even this one line deserves to be moved into lavc, since the fact that ffplay/ffmpeg were wrong for 5+ years means nobody can be expected to get it right.
> 
> Why it shouldn't moved in another function, provided by libavformat
> possibly covering all the heuristic so you *can* use it to get the pts
> but you can do w/out if you know what are you doing?
> 
> avcodec_decode_video(..)
> pts = avformat_guess_pts(avctx, avfctx, ...);

Well, this just adds a new field that can be read. The other two (pkt_pts, pkt_dts) aren't deprecated so they can still be used.

I think the advantage of using a field is that we don't have to decide what the function parameters are.
For instance, avfctx isn't really helpful for anything, not even r_frame_rate. It would be better (because of VFR) to save the last good frame duration and use it to fix sparse/duplicate times at the end.

And that certainly is something that should be done in libavformat... but it's much easier to code it against decoded frames first, and if the code only exists in implementation it can be removed eventually if nothing is left that needs such fixing up.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pts_logging.diff
Type: application/octet-stream
Size: 1984 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110218/334a82fb/attachment.obj>