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

[Michael Niedermayer]

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 ...

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.

previously we just used a timebase in which the durations couldnt be specified
and then defined repeat_pict based on tb/2. This of course wasnt correct ...


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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/ac496196/attachment.pgp>