[FFmpeg-devel] [RFC] - GXF Encoder fixes and HD support

[Reuben Martin]

Yo, back on Wednesday, September 08, 2010 Carl Eugen Hoyos was all like:
> Reuben Martin <reuben.m <at> gmail.com> writes:
> 
> > Comments, suggestions, code, flames all welcome.
> 
> I am sure work on this is very welcome, I can't really comment on the details,
> but here are a few cosmetic hints:
> 
> Please split the short patch again: I suspect the two hunks are unrelated (could
> you confirm that you can produce valid H264 gxf files only with this short
> change? If not, it should be part of one patch that adds H264 support.)

Forgot in my previous reply: This chunk of code is for decoding only. I can confirm that I can get it to play back, but the playback is not correct. The color-space conversion is not correct (the AVCi samples I have access to use yuvj420p color space) and it complains about too many reference frames:

[h264 @ 0x30c9f00] number of reference frames exceeds max (probably corrupt input), discarding one

If you want to look at the samples yourself, they can be found here: http://www.gvgdevelopers.com/concrete/products/k2/test_clips/ (any files with "AVCI50" in the file name)

-Reuben



> 
> > -    //{ CODEC_ID_NONE,  ,   18 }, /* Non compressed 24 bit audio */
> > +    //{ CODEC_ID_NONE    ,  18 }, /* Non compressed 24 bit audio */
> 
> > -                gxf->flags |= 0x00000040;
> >                  gxf->time_base = (AVRational){ 1, 50 };
> > +                gxf->flags |= 0x00000040;
> 
> Such changes are very unwelcome, please remove them before submitting (they make
> reviewing harder).
> 
> > -                    sc->media_type += 2;
> > -                    sc->track_type = 6;
> > -                    gxf->flags |= 0x00002000;
> > -                    media_info = 'E';
> ...
> > +                        sc->media_type += 2;
> > +                        gxf->flags |= 0x00002000;
> > +                        media_info = 'E';
> > +                        sc->track_type = 6;
> 
> Please do not re-indent in this case to make the patch smaller. (Re-indentation
> goes into a separate commit.)
> 
> > +#if 0
> > +            case CODEC_ID_H264:
> > +                    sc->media_type = 26;
> > +                    gxf->flags |=  0x02000000;
> > +                    media_info = 'I';
> > +                    sc->track_type = 11;
> > +                break;
> > +#endif
> 
> If this does not work, please remove it (including the hunk in the other file),
> dead code often just rots.
> 
> I don't know much about gxf, but if you try to fix different issues here (it
> looks as if), please try to send one patch per issue (not per file).
> 
> Thank you, Carl Eugen
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>