[FFmpeg-devel] [PATCH] CrystalHD decoder support v3

[Philip Langdale]

 On Tue, 1 Feb 2011 15:17:37 +0100, Benoit Fouet <benoit.fouet at free.fr> 
 wrote:
> Hi,
>
> mostly a naive pass on the code.

 Thanks for taking a look!

 I will address your feedback in the next patch revision. For now, I'll 
 just
 respond to two points.
 
>
>> +static inline int extract_sps_pps_from_avcc(CHDContext *priv,
>> +                                            uint8_t *data,
>> +                                            uint32_t data_size)
>> +{
>> [...]
>> +}
>
> Do you think some code could be shared between this and and the mp4
> to annex B bitstream filter?
>

 Ah ha! I think so, if I'm reading libavformat/sdp.c correctly. It looks
 like that is using it to rewrite extradata and nothing else; if so, 
 that's
 what I need and I can just use it as-is and forget about the broadcom 
 code.

>
> For the following AVCodec definitions, shouldn't they depend on
> config.h defines at some point?
> e.g.:
>
> #if CONFIG_H264_CRYSTALHD_DECODER
>> +AVCodec ff_h264_crystalhd_decoder = {
>> +    "h264_crystalhd",
>> +    AVMEDIA_TYPE_VIDEO,
>> +    CODEC_ID_H264,
>> +    sizeof(CHDContext),
>> +    init,
>> +    NULL,
>> +    uninit,
>> +    decode,
>> +    CODEC_CAP_DR1 | CODEC_CAP_DELAY,
>> +    .flush = flush,
>> +    .long_name = NULL_IF_CONFIG_SMALL("H.264 / AVC / MPEG-4 AVC / 
>> MPEG-4 part 10 (CrystalHD acceleration)"),
>> +    .pix_fmts = (const enum PixelFormat[]){PIX_FMT_YUYV422, 
>> PIX_FMT_NONE},
>> +};
>> +
>
> #endif
> and so on

 I guess so? But in practice, you either get all of them or none of them 
 as there are no
 extra dependencies - if crystalhd.c compiles at all, you get all the 
 decoders.

 --phil