[FFmpeg-devel] [PATCH] H.264 timestamps in h264_parser - complete set

[Ivan Schreter]

Michael Niedermayer wrote:
> I ve just added h264 to tb_unreliable()
>   
So, here the new patches:

avcodec_timebase: change duration computation to use time_base instead 
of TB/2.
h264_timebase: correct time_base of H.264 and repeat_pict.

Further, I suppose, MPEG timebase should be adjusted as well, see 
mpeg_timebase (actually your patch), to handle repeat_pict correctly.

I also noticed that ffplay computes frame delay using old formula with 
1/2 fps. So I suppose, this should be changed as well (including 
increasing AVFrame.repeat_pict analogously to AVPacket.repeat_pict for 
H.264 and MPEG). Correct? If so, I could make a patch for it as well.

In general, I wonder if repeat_pict (both in AVFrame and AVPacket) 
should not be renamed to frame_duration on next major version bump and 
increased by 1 everywhere...

Timestamp computation is now also adjusted to your proposed names:

avcodec_timestamp: add timestamp computation, if values exported from codec.
h264_timestamp: timestamp parameter export from H.264.

One problem remains, though: FPS detection doesn't take field pictures 
into account. So if the format returns frames containing field pictures 
with 1/2 frame duration (or better to say, with DTS distance of 1/2 
frame duration), then it will detect 2*fps instead of correct fps.

I'd propose adding a flag field_picture_flag to AVPacket (and pass it 
from the parser) to notify the user about field pictures, so it can 
correctly compute frame rate. What do you think? Or do you have a better 
idea?

 > just added AVFMT_VARIABLE_FPS

Shouldn't then mpegts have variable FPS as well? See mpegts_varframe patch.


>> [...]
>> Another question, how does the player (or user application) know, after 
>> avcodec_decode_video() returns a picture, which timestamp and duration 
>> this picture has (OK, duration is expressed in repeat_pict, but where 
>> does it get the PTS - pts field in picture is currently not filled at 
>> all or wrong)? Pictures are being reordered for MPEG/H.264... I'm 
>> afraid, this needs to be fixed as well.
>>     
>
> see reordered_opaque
>   
Thanks, I was looking for something like this.

Regards,

Ivan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: avcodec_timebase.patch
Type: text/x-patch
Size: 1429 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090224/e6a758a4/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: h264_timebase.patch
Type: text/x-patch
Size: 2537 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090224/e6a758a4/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpeg_timebase.patch
Type: text/x-patch
Size: 2339 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090224/e6a758a4/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avcodec_timestamp.patch
Type: text/x-patch
Size: 5460 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090224/e6a758a4/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: h264_timestamp.patch
Type: text/x-patch
Size: 739 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090224/e6a758a4/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegts_varframe.patch
Type: text/x-patch
Size: 378 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090224/e6a758a4/attachment-0005.bin>