diff mbox series

[FFmpeg-devel] avutil/hdr_dynamic_metadata: allow av_dynamic_hdr_plus_to_t35() to accept an existing buffer

Message ID 20230401152205.296-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avutil/hdr_dynamic_metadata: allow av_dynamic_hdr_plus_to_t35() to accept an existing buffer | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer April 1, 2023, 3:22 p.m. UTC 
The function now accepts an existing buffer to avoid unnecessary allocations,
as well as only reporting the needed amount of bytes if you pass a NULL pointer
as input for data.
For this, both parameters become input and output, as well as making data
optional, and the max payload size define is made public so library users can
just use that f they want to use their own buffers, knowing it will be enough
for any input AVDynamicHDRPlus.

Signed-off-by: James Almer <jamrial@gmail.com>
---
Missing version bump (Or maybe i can just not bump since this function is
recent enough?) and APIChanges entry.

 libavutil/hdr_dynamic_metadata.c | 25 +++++++++++++++++--------
 libavutil/hdr_dynamic_metadata.h | 14 +++++++++++---
 2 files changed, 28 insertions(+), 11 deletions(-)

Comments

James Almer April 3, 2023, 6:26 p.m. UTC | #1 
On 4/1/2023 12:22 PM, James Almer wrote:
> The function now accepts an existing buffer to avoid unnecessary allocations,
> as well as only reporting the needed amount of bytes if you pass a NULL pointer
> as input for data.
> For this, both parameters become input and output, as well as making data
> optional, and the max payload size define is made public so library users can
> just use that f they want to use their own buffers, knowing it will be enough
> for any input AVDynamicHDRPlus.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Missing version bump (Or maybe i can just not bump since this function is
> recent enough?) and APIChanges entry.

I'll apply this with a version bump soon.
diff mbox series

Patch

diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index d458788c32..5b56448f0d 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -24,8 +24,6 @@ 
 #include "libavcodec/get_bits.h"
 #include "libavcodec/put_bits.h"
 
-#define T35_PAYLOAD_MAX_SIZE 907
-
 static const int64_t luminance_den = 1;
 static const int32_t peak_luminance_den = 15;
 static const int64_t rgb_den = 100000;
@@ -62,14 +60,14 @@  AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
 int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
                                  size_t size)
 {
-    uint8_t padded_buf[T35_PAYLOAD_MAX_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
+    uint8_t padded_buf[AV_HDR_PLUS_MAX_PAYLOAD_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
     GetBitContext gbc, *gb = &gbc;
     int ret;
 
     if (!s)
         return AVERROR(ENOMEM);
 
-    if (size > T35_PAYLOAD_MAX_SIZE)
+    if (size > AV_HDR_PLUS_MAX_PAYLOAD_SIZE)
         return AVERROR(EINVAL);
 
     memcpy(padded_buf, data, size);
@@ -243,8 +241,10 @@  int av_dynamic_hdr_plus_to_t35(const AVDynamicHDRPlus *s, uint8_t **data, size_t
     size_t size_bits, size_bytes;
     PutBitContext pbc, *pb = &pbc;
 
-    if (!s || !data)
+    if (!s)
         return AVERROR(EINVAL);
+    if ((!data || *data) && !size)
+       return AVERROR(EINVAL);
 
     /**
      * Buffer size per CTA-861-H p.253-254:
@@ -296,9 +296,18 @@  int av_dynamic_hdr_plus_to_t35(const AVDynamicHDRPlus *s, uint8_t **data, size_t
 
     size_bytes = (size_bits + 7) / 8;
 
-    buf = av_mallocz(size_bytes);
-    if (!buf)
-        return AVERROR(ENOMEM);
+    if (!data) {
+        *size = size_bytes;
+        return 0;
+    } else if (*data) {
+        if (*size < size_bytes)
+            return AVERROR_BUFFER_TOO_SMALL;
+        buf = *data;
+    } else {
+        buf = av_malloc(size_bytes);
+        if (!buf)
+            return AVERROR(ENOMEM);
+    }
 
     init_put_bits(pb, buf, size_bytes);
 
diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
index 771bb8f468..f3513807f6 100644
--- a/libavutil/hdr_dynamic_metadata.h
+++ b/libavutil/hdr_dynamic_metadata.h
@@ -353,13 +353,21 @@  AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
 int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
                                  size_t size);
 
+#define AV_HDR_PLUS_MAX_PAYLOAD_SIZE 907
+
 /**
  * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer,
  * excluding the first 48 bytes of the header, and beginning with the application mode.
  * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
- * @param data A pointer to a byte buffer to be allocated and filled
- *             with the serialized metadata.
- * @param size A pointer to a size to be set to the returned buffer's size (optional).
+ * @param data[in,out] A pointer to pointer to a byte buffer to be filled with the
+ *                     serialized metadata.
+ *                     If *data is NULL, a buffer be will be allocated and a pointer to
+ *                     it stored in its place. The caller takes ownership of the buffer.
+ *                     May be NULL, in which case the function will only store the
+ *                     required buffer size in *size.
+ * @param size[in,out] A pointer to a size to be set to the returned buffer's size.
+ *                     If *data is not NULL, *size must contain the size of the input
+ *                     buffer. May be NULL only if *data is NULL.
  *
  * @return >= 0 on success. Otherwise, returns the appropriate AVERROR.
  */