[FFmpeg-devel] [FFmpeg-cvslog] libavcodec/libx264: add user data unregistered SEI encoding

[Brad Hards]

On Tuesday, 19 October 2021 9:53:50 AM AEDT James Almer wrote:
> > The problem i guess is in the following line:
> > 
> > sei_payload->payload = side_data->data;
> > 
> > Which should be an av_memdup() or similar, because side_data->data is
> > the frame's side data, and by the time the encoder tries to do something
> > with it, it may have been freed.
> > Also, this code should set x4->pic.extra_sei.sei_free to av_free().
> 
> The following should fix it
....

I've tried to do a similar fix for libx265, but I don't know how to handle
the cleanup.  This is what I have so far:

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 7dd70a3450..ee27e67af1 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -52,9 +52,6 @@ typedef struct libx265Context {
     char *profile;
     AVDictionary *x265_opts;
 
-    void *sei_data;
-    int sei_data_size;
-
     /**
      * If the encoder does not support ROI then warn the first time we
      * encounter a frame with ROI side data.
@@ -82,7 +79,6 @@ static av_cold int libx265_encode_close(AVCodecContext *avctx)
     libx265Context *ctx = avctx->priv_data;
 
     ctx->api->param_free(ctx->params);
-    av_freep(&ctx->sei_data);
 
     if (ctx->encoder)
         ctx->api->encoder_close(ctx->encoder);
@@ -545,24 +541,30 @@ static int libx265_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 
         for (i = 0; i < pic->nb_side_data; i++) {
             AVFrameSideData *side_data = pic->side_data[i];
+            unsigned int sei_data_size = 0;
             void *tmp;
             x265_sei_payload *sei_payload;
 
             if (side_data->type != AV_FRAME_DATA_SEI_UNREGISTERED)
                 continue;
 
-            tmp = av_fast_realloc(ctx->sei_data,
-                                  &ctx->sei_data_size,
+            tmp = av_fast_realloc(sei->payloads, 
+                                  &sei_data_size,
                                   (sei->numPayloads + 1) * sizeof(*sei_payload));
             if (!tmp) {
                 av_freep(&x265pic.userData);
                 av_freep(&x265pic.quantOffsets);
                 return AVERROR(ENOMEM);
             }
-            ctx->sei_data = tmp;
-            sei->payloads = ctx->sei_data;
+            sei->payloads = tmp;
+            // Don't know how to av_free() sei here...
             sei_payload = &sei->payloads[sei->numPayloads];
-            sei_payload->payload = side_data->data;
+            sei_payload->payload = av_memdup(side_data->data, side_data->size);
+            if (!sei_payload->payload) {
+                av_freep(&x265pic.userData);
+                av_freep(&x265pic.quantOffsets);
+                return AVERROR(ENOMEM);
+            }
             sei_payload->payloadSize = side_data->size;
             /* Equal to libx265 USER_DATA_UNREGISTERED */
             sei_payload->payloadType = SEI_TYPE_USER_DATA_UNREGISTERED;
@@ -573,6 +575,7 @@ static int libx265_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     ret = ctx->api->encoder_encode(ctx->encoder, &nal, &nnal,
                                    pic ? &x265pic : NULL, &x265pic_out);
 
+    av_freep(&x265pic.userData);
     av_freep(&x265pic.quantOffsets);
 
     if (ret < 0)

Would appreciate any suggestions (or a fix).

Brad