[FFmpeg-devel] [PATCH 8/10] GXF Encoder Fixes and HD support (patch broken up)

Reuben Martin reuben.m
Fri Sep 10 00:09:38 CEST 2010


Yo, back on Thursday, September 09, 2010 Baptiste Coudurier was all like:
> On 09/08/2010 06:40 PM, Reuben Martin wrote:
> > Yo, back on Wednesday, September 08, 2010 Reuben Martin was all like:
> >> 08-gxf__HD_support.patch
> >>
> >> FEATURE: Adds support for HD MPEG2 content and to a lesser extent DVCPROHD.  Also makes changes in setting the frame/field rates depending on the time base rather than line-height. This is the patch needed the most input since I'm not sure how best to deal with detecting interlaced video streams.
> >>
> >>
> >
> >
> > 08-gxf__HD_support.patch
> >
> >
> > --- ffmpeg-old/libavformat/gxfenc.c	2010-09-08 17:12:41.777000111 -0500
> > +++ ffmpeg-new/libavformat/gxfenc.c	2010-09-08 17:23:48.446000108 -0500
> > @@ -81,6 +81,8 @@
> >   static const AVCodecTag gxf_media_types[] = {
> >       { CODEC_ID_MJPEG     ,   3 }, /* NTSC */
> >       { CODEC_ID_MJPEG     ,   4 }, /* PAL */
> > +    //{ CODEC_ID_NONE,   ,   7 }, /* TimeCode-525 */
> > +    //{ CODEC_ID_NONE,   ,   8 }, /* TimeCode-625 */
> >       { CODEC_ID_PCM_S24LE ,   9 },
> >       { CODEC_ID_PCM_S16LE ,  10 },
> >       { CODEC_ID_MPEG2VIDEO,  11 }, /* NTSC */
> > @@ -91,9 +93,14 @@
> >       { CODEC_ID_DVVIDEO   ,  16 }, /* 50M PAL */
> >       { CODEC_ID_AC3       ,  17 },
> >       //{ CODEC_ID_NONE,  ,   18 }, /* Non compressed 24 bit audio */
> > +    //{ CODEC_ID_NONE,   ,  19 }, /* Non compressed 32 bit audio */
> >       { CODEC_ID_MPEG2VIDEO,  20 }, /* MPEG HD */
> > +    //{ CODEC_ID_NONE,   ,  21 }, /* Ancillary Data (track_type=8, meida_info='N') */
> >       { CODEC_ID_MPEG1VIDEO,  22 }, /* NTSC */
> >       { CODEC_ID_MPEG1VIDEO,  23 }, /* PAL */
> > +    //{ CODEC_ID_NONE,   ,  24 }, /* TimeCode */
> > +    { CODEC_ID_DVVIDEO   ,  25 }, /* 100M DVCPROHD */
> > +    //{ CODEC_ID_H264    ,  26 }, /* AVCi 50 / 100 (H264 Intra) */
> >       { CODEC_ID_NONE,         0 },
> >   };
> 
> I don't think we really need all the unused definitions.
> 
>  > [...]
>  >
> > +
> > +            if (fabs(av_q2d(st->codec->time_base) - 1/60.0) == 0.0) {
> 
> < 0.0001 like the other checks.

Doing that would present a problem as it is close enough to the values from 6000/1001 that it would create false positives.

However, I'm not sure if 1/60 and 1/30 are really needed. If they are intended to differentiate between two distinct frame/field rates then they should stay, but if they are the same thing, but one is used for drop-frame and the other is used for non-drop-frame timecode, then we could get rid of them since ffmpeg only writes the non-drop-frame timecode. If you have access to the specs perhaps you could look that up.

> 
>  > [...]
>  >
> > +            } else if (fabs(av_q2d(st->codec->time_base) - 1/30.0) == 0.0) {
> > +                sc->frame_rate_index = 4;
> > +                if (sc->fields == 1) {
> > +                    sc->sample_rate = 30;
> > +                    gxf->time_base = (AVRational){ 1, 30 };
> > +                    gxf->flags |= 0x00000400;
> > +                } else {
> > +                    sc->sample_rate = 60;
> > +                    gxf->time_base = (AVRational){ 1, 60 };
> > +                    gxf->flags |= 0x00000080;
> > +                }
> > +            } else if (fabs(av_q2d(st->codec->time_base) - 1001/30000.0)<  0.0001) {
> > +                sc->frame_rate_index = 5;
> > +                if (sc->fields == 1) {
> > +                    sc->sample_rate = 30;
> > +                    gxf->time_base = (AVRational){ 1001, 30000 };
> > +                    gxf->flags |= 0x00000400;
> > +                } else {
> > +                    sc->sample_rate = 60;
> > +                    gxf->time_base = (AVRational){ 1001, 60000 };
> > +                    gxf->flags |= 0x00000080;
> > +                }
> > +            } else if (fabs(av_q2d(st->codec->time_base) - 1/25.0)<  0.0001) {
> > +                sc->frame_rate_index = 6;
> > +                sc->media_type++;
> > +                if (sc->fields == 1) {
> > +                    sc->sample_rate = 25;
> > +                    gxf->time_base = (AVRational){ 1, 25 };
> > +                    gxf->flags |= 0x00000200;
> > +                } else {
> > +                    sc->sample_rate = 50;
> > +                    gxf->time_base = (AVRational){ 1, 50 };
> > +                    gxf->flags |= 0x00000040;
> > +                }
> 
> Humm you are changing the values for NTSC and PAL.
> IIRC GXF always count the field rate even if video is progressive 
> according to specs. This might need double check though.
> 
> [...]
> 
> 

I'm not really sure what is "correct" here. Take this sample from GrassValley for instance: http://www.gvgdevelopers.com/K2DevGuide/Clips/K2_HD_MPEG_colorbar.gxf

It is interlaced NTSC, but both it's frame rate and sample rate are set to 30.

There's also a 720p that has 60 both frame rate and sample rate. Which fully possible for DVCPROHD cams, but there you have the frame rate and field rate set the same also. http://www.gvgdevelopers.com/K2DevGuide/Clips/525_DV100_720p_colorbar.gxf

Further more, I'm suspicious of the frame rates I get from ffmpeg itself. If I dump an mpeg2video out to a raw mpeg2video stream (interlaced field rate of 60/1.001) then use it as an input stream to copy into a gxf file, ffmpeg's terminal output (from libavformat/utils.c) says the tbc value is 60/1.001, but calculating the tbc value within gxfenc.c I get 30/1.001. I suspect that somewhere the timebase is adjusted to reflect frame rate rather than field rate, although I haven't tried to figure out what is going on there.

Insight from the official spec would be helpful here as it is very possible that GrassValley's samples violate the spec.

-Reuben



More information about the ffmpeg-devel mailing list