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

[Reuben Martin]

On Wed, Sep 8, 2010 at 1:05 PM, Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:
> 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:
>

Yes, I plan to chop up the diffs according to bugs fixed and features
added and fix cosmetic things. Right now my concern isn't really about
cosmetics, but rather on getting feedback about functionality. GXF
implementations elsewhere seem to be very picky about certain things,
totally ignore others, and sometime don't seem to adhere to the spec
very well. My biggest concern is with breaking things that currently
work with ffmpeg's gxf. Although, if it helps in getting feedback I
can go ahead and chop the diffs up now.


> 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.)
>
>> - ? ?//{ 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.

This is disabled because I have no reference for writing the UMF media
packet headers for AVCi streams. Although I guess I could just write
an empty one like MJPEG currently does. I have no access to the
documents (and as I have no current use for AVCi myself at the moment,
I'm not too eager to pony up the cash to do so). I figured out what
some of the bit values were elsewhere by going through and toggling
them one by one and seeing how the tstream parser interpreted it. But
AVCi support is apparently a fairly recent extension, because the
tstream parser doesn't recognize it. I got the media_type, flags,
media_info, and track_type values by opening up some AVCi GXF samples
provided by GrassValley. But the UMF bit flag attributes have no names
or labels associated with them within the GXF file, so I have no idea
what features the bit values would map to. I would enable this code if
I can write the UMF media packet header.


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