[FFmpeg-cvslog] r17572 - in trunk/libavcodec: mpeg12.c mpegvideo_parser.c

Baptiste Coudurier baptiste.coudurier
Wed Feb 25 21:46:14 CET 2009


On 2/25/2009 11:59 AM, Michael Niedermayer wrote:
> On Wed, Feb 25, 2009 at 11:24:10AM -0800, Baptiste Coudurier wrote:
>> On 2/25/2009 11:08 AM, Michael Niedermayer wrote:
>>> On Wed, Feb 25, 2009 at 10:28:11AM -0800, Baptiste Coudurier wrote:
>>>> On 2/25/2009 2:52 AM, Michael Niedermayer wrote:
>>>>> On Wed, Feb 25, 2009 at 01:06:02AM -0800, Baptiste Coudurier wrote:
>>>>>> On 2/24/2009 12:23 PM, cehoyos wrote:
>>>>>>> Author: cehoyos
>>>>>>> Date: Tue Feb 24 21:23:19 2009
>>>>>>> New Revision: 17572
>>>>>>>
>>>>>>> Log:
>>>>>>> Correct time_base and repeat_pict for MPEG2 video.
>>>>>>>
>>>>>>> Patch by Ivan Schreter, schreter gmx net
>>>>>>>
>>>>>>> Modified:
>>>>>>>    trunk/libavcodec/mpeg12.c
>>>>>>>    trunk/libavcodec/mpegvideo_parser.c
>>>>>>>
>>>>>>> Modified: trunk/libavcodec/mpeg12.c
>>>>>>> ==============================================================================
>>>>>>> --- trunk/libavcodec/mpeg12.c	Tue Feb 24 21:19:59 2009	(r17571)
>>>>>>> +++ trunk/libavcodec/mpeg12.c	Tue Feb 24 21:23:19 2009	(r17572)
>>>>>>> @@ -1275,7 +1275,7 @@ static int mpeg_decode_postinit(AVCodecC
>>>>>>>              av_reduce(
>>>>>>>                  &s->avctx->time_base.den,
>>>>>>>                  &s->avctx->time_base.num,
>>>>>>> -                ff_frame_rate_tab[s->frame_rate_index].num * s1->frame_rate_ext.num,
>>>>>>> +                ff_frame_rate_tab[s->frame_rate_index].num * s1->frame_rate_ext.num*2,
>>>>>>>                  ff_frame_rate_tab[s->frame_rate_index].den * s1->frame_rate_ext.den,
>>>>>>>                  1<<30);
>>>>>>>          //MPEG-2 aspect
>>>>>>>
>>>>>>> Modified: trunk/libavcodec/mpegvideo_parser.c
>>>>>>> ==============================================================================
>>>>>>> --- trunk/libavcodec/mpegvideo_parser.c	Tue Feb 24 21:19:59 2009	(r17571)
>>>>>>> +++ trunk/libavcodec/mpegvideo_parser.c	Tue Feb 24 21:23:19 2009	(r17572)
>>>>>>> @@ -81,7 +81,7 @@ static void mpegvideo_extract_headers(AV
>>>>>>>                          pc->height |=( vert_size_ext << 12);
>>>>>>>                          avctx->bit_rate += (bit_rate_ext << 18) * 400;
>>>>>>>                          avcodec_set_dimensions(avctx, pc->width, pc->height);
>>>>>>> -                        avctx->time_base.den = pc->frame_rate.den * (frame_rate_ext_n + 1);
>>>>>>> +                        avctx->time_base.den = pc->frame_rate.den * (frame_rate_ext_n + 1) * 2;
>>>>>>>                          avctx->time_base.num = pc->frame_rate.num * (frame_rate_ext_d + 1);
>>>>>>>                          avctx->codec_id = CODEC_ID_MPEG2VIDEO;
>>>>>>>                          avctx->sub_id = 2; /* forces MPEG2 */
>>>>>> I'm pretty sure this commit broke codec frame rate parsing for MPEG-2 in
>>>>>> MXF.
>>>>>>
>>>>>> Seems stream 0 codec frame rate differs from container frame rate: 50.00
>>>>>> (50/1) -> 25.00 (25/1)
>>>>>> Input #0, mxf, from 'test.mxf':
>>>>>>   Duration: 00:00:18.20, start: 0.000000, bitrate: 2086 kb/s
>>>>>>     Stream #0.0: Video: mpeg2video, yuv420p, 320x240 [PAR 1:1 DAR 4:3],
>>>>>> 104857 kb/s, 25 tbr, 25 tbn, 50 tbc
>>>>>>     Stream #0.1: Audio: pcm_s16le, 48000 Hz, stereo, s16, 1536 kb/s
>>>>>> At least one output file must be specified
>>>>> elaborate on what is wrong please
>>>> 50 tbc is wrong, should be 25.
>>>>
>>>>>> Worked fine before. You can just create a .mxf with FFmpeg:
>>>>>> ffmpeg -i <file> -ar 48000 test.mxf
>>>>>>
>>>>>> If I remove "* 2" everything is fine again.
>>>>> cant be, durations (and thus timestamps) in mpeg2 are specified in
>>>>> half framerate units thus timebase has to be half of 1/framerate.
>>>> Well, if I remove * 2 it is 25 again.
>>>>
>>>> Can you please explain how can durations and timestamps be specified in
>>>> half framerate ? Do you mean when it is coded as separate fields ?
>>>> Here one frame at 25 in PS/TS has 3600 duration, this is not half frame
>>>> rate.
>>> frames can be output as 3 fields, its done in telecine and its generally
>>> progressive not interlaced material.
>>> To specify the stored durations exacly a 1/50 timebase is needed.
>>> And we cant change the timebase once telecine is encountered and it could
>>> be later in the movie ...
>> I think repeat_pict is related to decoding, not demuxing. Besides, a
>> decoder can choose to ignore repeat_pict in the case of telecine, to
>> output progressive material. So if decoder decides to honor repeat pict,
>> it will do after decoding, right ?
> 
> our decoder doesnt really honor repeat_pict beyond exporting its value.
> 
> 
>> Duration will depend on what the decoder will do, libavformat cannot
>> predict anything in this case.
> 
> in mpeg-ps and mpeg-ts you have a timetamp once every 0.7 second assuming
> there was no packet loss hitting that timestamp.
> the only way to know the pts/dts/when to display the frames between is
> repeat_pict
> In that sense repeat_pict is relevant for decoding & demuxing.
> 
> and ignoring repeat_pict entirely does not work because that way neither
> demuxer nor decoder would have timestamps, if the timestamps where taken
> tb or 2*tb apart it would fail badly for telecine that has a value between
> for the average

Does repeat_pict is equal to repeat_first_field ? Because in this case,
name is not appropriate and should be changed.

Also when progressive sequence is set, whole frame must be duplicated
per specs. Is this taken into account ?

Now every mpeg2 video codec spits:
Seems stream 0 codec frame rate differs from container frame rate: 50.00
(50/1) -> 25.00 (25/1)

This is really annoying.

Also when stream copying muxer gets 1/50 as timebase, this literally
broke muxers checking codec time base for parameters (MXF).

In the later how the muxer can make the difference between 25 and 50 fps
? Will we get 100 ? If codec is not mpeg-2 then muxers should check for
a different value ? This looks odd.

How can the application fetch the "real" value of frame rate as stored
in mpeg-2 essence ?

I'd say add a new field to mention this information if you want, but
keep AVCodecContext->time_base as it was before, aka value of frame rate
as stored in mpeg-2 essence.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org




More information about the ffmpeg-cvslog mailing list