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

[Baptiste Coudurier]

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 ?

Duration will depend on what the decoder will do, libavformat cannot
predict anything in this case.

> iam not happy about this either, ideally mpeg would not support these idiotic
> field repeats but rather store true timestamps and leave it to the player to
> repeat if it needs to.

Well, I'm ok to support it, but in this case this has a very bad side
effect, and this changes heavily the behaviour of libavformat.

[...]

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