[FFmpeg-devel] [PATCH v7 07/14] avutil/frame: add helper for adding side data w/ AVBufferRef to array

[James Almer]

On 3/1/2024 3:19 PM, Anton Khirnov wrote:
> Quoting Jan Ekström (2024-02-29 17:42:54)
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index 47d0096bc4..908fd4a90d 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -1081,6 +1081,26 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
>>                                           enum AVFrameSideDataType type,
>>                                           size_t size, unsigned int flags);
>>   
>> +/**
>> + * Add a new side data entry to a set from an existing AVBufferRef.
>> + *
>> + * @param sd    pointer to array of side data to which to add another entry.
>> + * @param nb_sd pointer to an integer containing the number of entries in
>> + *              the array.
>> + * @param type  type of the added side data
>> + * @param buf   AVBufferRef for which a new reference will be made
> 
> This seems inconsistent with av_frame_new_side_data_from_buf(), which
> takes ownership of the buffer reference passed to it.
> Is there a strong reason to do it differently here?

I asked for it. Taking ownership of the AVBufferRef and then expecting 
the caller to discard the pointer to it is awkward, and unlike any other 
function using AVBufferRef we have in the tree.

The alternative, to keep the behavior of taking ownership of the passed 
on reference, is to have this function take a pointer to pointer, and 
clearing it on success.