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

[Michael Niedermayer]

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


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090225/745361dc/attachment.pgp>