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

[Anton Khirnov]

Quoting James Almer (2024-03-01 21:42:28)
> 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.

That seems preferable to me, in most cases the caller does not want to
keep a reference.

-- 
Anton Khirnov