[FFmpeg-devel] [PATCH 3/5] avcodec/bsf: Add list BSF API

Jan Sebechlebsky sebechlebskyjan at gmail.com
Fri Aug 5 14:40:58 EEST 2016


Hello Nicolas,

On 08/02/2016 05:14 PM, Nicolas George wrote:
>
>> +AVBSFList *av_bsf_list_alloc(void);
> This is personal, but for new APIs, I like "int foo_alloc(Foo **rfoo)"
> better than "Foo *foo_alloc(void)": that way, the caller can forward the
> error code instead of guessing it is ENOMEM.
I left that as it is, since James commented this is more common way.
>
>> +
>> +typedef struct AVBSFList {
>> +    AVBSFContext **bsf_lst;
>> +    int ctx_nr;
>> +} AVBSFList;
> I think it would be possible to use the same structure for AVBSFList and
> BSFListContext. Sure, AVBSFList would have extra fields, but that is no
> matter, since its only use is to be eventually upgraded into a
> BSFListContext.
>
> The benefit would be to avoid a malloc()/copy/free() cycle: just declare
> .priv_data_size = 0 to prevent lavc from allocating the private structure,
> and set priv_data directly.
I think it is nicer the way it is - I do not like an idea of manually 
assigning private data when priv_data_size will be set to 0, it seems 
like something which can potentially create problem in future. Also the 
overhead is negligible here, since bsf list finalization will be done 
only once or few times per whole transcoding.

I've fixed all other issues you suggested.

Thanks for review!

Regards,
Jan


More information about the ffmpeg-devel mailing list