On 3/25/2024 6:02 PM, Andreas Rheinhardt wrote:
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 <jamr...@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.

Just because there's a frame side data type for MDM does not make the new alloc function redundant. The frame side data API is one API wrapping others. You don't see code using the AVHash API when they only care about AVMD5. MDM is and should be standalone. av_mastering_display_metadata_alloc() should be deprecated and removed after this, too. That will ensure we still have a single symbol for this.

Regarding the av_*_create_side_data() functions, they were never needed. They are a convenient wrapper/shortcut, and your av_frame_side_data_allocate() would not really replace them as you'd still need to use av_frame_add_side_data() afterwards.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to