[FFmpeg-devel] [PATCH] IFF: Add the HAM stuff

Sebastian Vater cdgs.basty
Sun May 16 02:18:28 CEST 2010


Stefano Sabatini a ?crit :
> On date Sunday 2010-05-16 00:40:21 +0200, Sebastian Vater encoded:
>   
>> Stefano Sabatini a ?crit :
>>     
>>>> +/**
>>>> + * Extracts the IFF extra context and updates internal
>>>> + * decoder structures.
>>>> + *
>>>> + * @param avctx the AVCodecContext where to extract extra context to
>>>> + * @param avpkt the AVPacket to extract extra context from
>>>> + *
>>>>     
>>>>         
>>> Nit+++: no need for this empty newline
>>>   
>>>       
>> Which empty new line? They're just cosmetics like in the other
>> functions, too. So you see return value separated to args.
>>     
>
> Well it's a level 3 nit, so it's really not important if you prefer
> that way (just in the most doxies in FFmpeg there is no empty newline
> before @return).
>   

I'ld prefer it with a newline, I simply think it's more comfortable to me.

> Patch looks fine to me, I suppose it has been tested and works, if no
> one has more comments I'll apply it in few days.
>   

Yes, I did a comprehensive test both on big and little endian, I tested
as well as grayscale and RGB (grayscale was tested by changing CMAP
identifier to CMAp...

> Keep up the excellent work!
>   

Yeah, thank you! I'll keep it on! Just review the new one I just
submitted. It adds with a very minimal overhead the same to
avctx->extradata, too.

When we have to break backward compatibility anyway, then we should make
avctx handling of custom data, too.

Before we later detect, that some data must be known at decode_init
time, better do it the right way straight ahead. ;-)

-- 

Best regards,
                   :-) Basty/CDGS (-:




More information about the ffmpeg-devel mailing list