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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to