[FFmpeg-devel] [PATCH 4/6 v2] avutil/mastering_display_metadata: add a new allocator function that returns a size

[Andreas Rheinhardt]

James Almer:
> On 3/25/2024 5:40 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> av_mastering_display_metadata_alloc() is not useful in scenarios
>>> where you need to
>>> know the runtime size of AVMasteringDisplayMetadata.
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>   libavutil/mastering_display_metadata.c | 13 +++++++++++++
>>>   libavutil/mastering_display_metadata.h |  9 +++++++++
>>>   2 files changed, 22 insertions(+)
>>>
>>> diff --git a/libavutil/mastering_display_metadata.c
>>> b/libavutil/mastering_display_metadata.c
>>> index 6069347617..ea41f13f9d 100644
>>> --- a/libavutil/mastering_display_metadata.c
>>> +++ b/libavutil/mastering_display_metadata.c
>>> @@ -18,6 +18,7 @@
>>>    * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> 02110-1301 USA
>>>    */
>>>   +#include <stddef.h>
>>>   #include <stdint.h>
>>>   #include <string.h>
>>>   @@ -29,6 +30,18 @@ AVMasteringDisplayMetadata
>>> *av_mastering_display_metadata_alloc(void)
>>>       return av_mallocz(sizeof(AVMasteringDisplayMetadata));
>>>   }
>>>   +AVMasteringDisplayMetadata
>>> *av_mastering_display_metadata_alloc_size(size_t *size)
>>> +{
>>> +    AVMasteringDisplayMetadata *mastering =
>>> av_mallocz(sizeof(AVMasteringDisplayMetadata));
>>> +    if (!mastering)
>>> +        return NULL;
>>> +
>>> +    if (size)
>>> +        *size = sizeof(*mastering);
>>> +
>>> +    return mastering;
>>> +}
>>> +
>>>   AVMasteringDisplayMetadata
>>> *av_mastering_display_metadata_create_side_data(AVFrame *frame)
>>>   {
>>>       AVFrameSideData *side_data = av_frame_new_side_data(frame,
>>> diff --git a/libavutil/mastering_display_metadata.h
>>> b/libavutil/mastering_display_metadata.h
>>> index c23b07c3cd..52fcef9e37 100644
>>> --- a/libavutil/mastering_display_metadata.h
>>> +++ b/libavutil/mastering_display_metadata.h
>>> @@ -77,6 +77,15 @@ typedef struct AVMasteringDisplayMetadata {
>>>    */
>>>   AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc(void);
>>>   +/**
>>> + * Allocate an AVMasteringDisplayMetadata structure and set its
>>> fields to
>>> + * default values. The resulting struct can be freed using av_freep().
>>> + *
>>> + * @return An AVMasteringDisplayMetadata filled with default values
>>> or NULL
>>> + *         on failure.
>>> + */
>>> +AVMasteringDisplayMetadata
>>> *av_mastering_display_metadata_alloc_size(size_t *size);
>>> +
>>>   /**
>>>    * Allocate a complete AVMasteringDisplayMetadata and add it to the
>>> frame.
>>>    *
>>
>> Instead of this we should have a generic allocator like
>> void *av_frame_side_data_allocate(enum AVFrameSideDataType, size_t
>> *size, size_t elem_count), with the latter being used for the allocators
>> that allocate arrays (like AVRegionOfInterest); it has to be set to zero
>> for the others. This will also avoid creating new
>> av_*_create_side_data() functions.
> 
> I don't mind a function like that being added to simplify future
> additions, but this API is orthogonal to the frame side data one. It's
> also used in packets, for example, and right now lavf is using
> sizeof(AVMasteringDisplayMetadata) because
> av_mastering_display_metadata_alloc() is not useful.
> 

The API proposed by me is supposed to make API like
av_mastering_display_metadata_alloc_size() redundant and therefore these
two additions are not orthogonal.

- Andreas