diff mbox

[FFmpeg-devel,v9,00/14] encoder AVCodecContext configuration side data

Message ID 20240317200100.152097-1-jeebjp@gmail.com
State New
Headers show

Commit Message

Jan Ekström March 17, 2024, 8 p.m. UTC 
Differences to v8:
1. rebased on top of current master
2. Applied changes based on comments done regarding the AVCodecContext, such
   as added details to the docstring as well as renaming the variable to
   `encoder_side_data` (ref: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240229164307.3535613-10-jeebjp@gmail.com/ ).

Comparison URL (mostly configure and wrappers, avutil/frame.c):
https://github.com/jeeb/ffmpeg/compare/avcodec_cll_mdcv_side_data_v8..avcodec_cll_mdcv_side_data_v9

This patch set I've now been working for a while since I felt like it was weird
we couldn't pass through information such as static HDR metadata to encoders
from decoded input. This initial version adds the necessary framework, as well
as adds static HDR metadata support for libsvtav1, libx264 as well as libx265
wrappers.

An alternative to this would be to make encoders only properly initialize when
they receive the first AVFrame, but that seems to be a bigger, nastier change
than introducing an AVFrameSideDataSet in avctx as everything seems to
presume that extradata etc are available after opening the encoder.

Note: Any opinions on whether FFCodec or AVCodec should have
      handled_side_data list, so that if format specific generic logic is
      added, it could be checked whether the codec itself handles this side
      data? This could also be utilized to list handled side data from f.ex.
      `ffmpeg -h encoder=libsvtav1`.

Jan


Jan Ekström (14):
  avutil/frame: split side data list wiping out to non-AVFrame function
  avutil/frame: add helper for freeing arrays of side data
  avutil/frame: split side_data_from_buf to base and AVFrame func
  avutil/frame: split side data removal out to non-AVFrame function
  avutil/frame: add helper for adding side data to array
  avutil/frame: add helper for adding existing side data to array
  avutil/frame: add helper for adding side data w/ AVBufferRef to array
  avutil/frame: add helper for getting side data from array
  avcodec: add frame side data array to AVCodecContext
  avcodec: add helper for configuring AVCodecContext's frame side data
  ffmpeg: pass first video AVFrame's side data to encoder
  avcodec/libsvtav1: add support for writing out CLL and MDCV
  avcodec/libx264: add support for writing out CLL and MDCV
  avcodec/libx265: add support for writing out CLL and MDCV

 configure                       |   2 +
 fftools/ffmpeg_enc.c            |  10 ++
 libavcodec/avcodec.c            |  31 ++++++
 libavcodec/avcodec.h            |  33 ++++++
 libavcodec/libsvtav1.c          |  69 ++++++++++++
 libavcodec/libx264.c            |  80 ++++++++++++++
 libavcodec/libx265.c            |  89 ++++++++++++++++
 libavcodec/options.c            |   2 +
 libavutil/Makefile              |   1 +
 libavutil/frame.c               | 180 ++++++++++++++++++++++++++------
 libavutil/frame.h               |  88 ++++++++++++++++
 libavutil/tests/side_data_set.c | 103 ++++++++++++++++++
 tests/fate/enc_external.mak     |  15 +++
 tests/fate/libavutil.mak        |   4 +
 tests/ref/fate/libsvtav1-hdr10  |  14 +++
 tests/ref/fate/libx264-hdr10    |  15 +++
 tests/ref/fate/libx265-hdr10    |  16 +++
 tests/ref/fate/side_data_set    |  14 +++
 18 files changed, 736 insertions(+), 30 deletions(-)
 create mode 100644 libavutil/tests/side_data_set.c
 create mode 100644 tests/ref/fate/libsvtav1-hdr10
 create mode 100644 tests/ref/fate/libx264-hdr10
 create mode 100644 tests/ref/fate/libx265-hdr10
 create mode 100644 tests/ref/fate/side_data_set


Full diff based on the same base hash for v8->v9:
diff mbox

Patch

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 0ced87b946..3183952172 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -696,20 +696,21 @@  int avcodec_configure_side_data(AVCodecContext *avctx,
 
     if (!sd) {
         av_frame_side_data_free(
-            &avctx->frame_side_data, &avctx->nb_frame_side_data);
+            &avctx->encoder_side_data, &avctx->nb_encoder_side_data);
         return 0;
     }
 
-    if (nb_sd > 0 && nb_sd == avctx->nb_frame_side_data &&
-        sd == (const AVFrameSideData **)avctx->frame_side_data)
+    if (nb_sd > 0 && nb_sd == avctx->nb_encoder_side_data &&
+        sd == (const AVFrameSideData **)avctx->encoder_side_data)
         return AVERROR(EINVAL);
 
     for (int i = 0; i < nb_sd; i++) {
         int ret = av_frame_side_data_clone(
-            &avctx->frame_side_data, &avctx->nb_frame_side_data, sd[i], flags);
+            &avctx->encoder_side_data, &avctx->nb_encoder_side_data, sd[i],
+            flags);
         if (ret < 0) {
             av_frame_side_data_free(
-                &avctx->frame_side_data, &avctx->nb_frame_side_data);
+                &avctx->encoder_side_data, &avctx->nb_encoder_side_data);
             return ret;
         }
     }
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 46523698ae..01085cc3c8 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2064,12 +2064,16 @@  typedef struct AVCodecContext {
     unsigned nb_side_data_prefer_packet;
 
     /**
-     * Set containing static side data, such as HDR10 CLL / MDCV structures.
-     * - encoding: set by user
+     * Array containing static side data, such as HDR10 CLL / MDCV structures.
+     * Side data entries should be allocated by usage of helpers defined in
+     * libavutil/frame.h.
+     *
+     * - encoding: may be set by user before calling avcodec_open2() for
+     *             encoder configuration.
      * - decoding: unused
      */
-    AVFrameSideData  **frame_side_data;
-    int             nb_frame_side_data;
+    AVFrameSideData  **encoder_side_data;
+    int             nb_encoder_side_data;
 } AVCodecContext;
 
 /**
diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
index 66fa019aa0..73bfdba3cf 100644
--- a/libavcodec/libsvtav1.c
+++ b/libavcodec/libsvtav1.c
@@ -181,12 +181,13 @@  static void handle_side_data(AVCodecContext *avctx,
 {
     const AVFrameSideData *cll_sd =
         av_frame_side_data_get(
-            (const AVFrameSideData **)avctx->frame_side_data,
-            avctx->nb_frame_side_data, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+            (const AVFrameSideData **)avctx->encoder_side_data,
+            avctx->nb_encoder_side_data, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
     const AVFrameSideData *mdcv_sd =
         av_frame_side_data_get(
-            (const AVFrameSideData **)avctx->frame_side_data,
-            avctx->nb_frame_side_data, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+            (const AVFrameSideData **)avctx->encoder_side_data,
+            avctx->nb_encoder_side_data,
+            AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
 
     if (cll_sd) {
         const AVContentLightMetadata *cll =
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index afeb285487..2a8d71cd0b 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -907,12 +907,13 @@  static void handle_side_data(AVCodecContext *avctx, x264_param_t *params)
 #if CONFIG_LIBX264_HDR10
     const AVFrameSideData *cll_sd =
         av_frame_side_data_get(
-            (const AVFrameSideData **)avctx->frame_side_data,
-            avctx->nb_frame_side_data, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+            (const AVFrameSideData **)avctx->encoder_side_data,
+            avctx->nb_encoder_side_data, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
     const AVFrameSideData *mdcv_sd =
         av_frame_side_data_get(
-            (const AVFrameSideData **)avctx->frame_side_data,
-            avctx->nb_frame_side_data, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+            (const AVFrameSideData **)avctx->encoder_side_data,
+            avctx->nb_encoder_side_data,
+            AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
 
     if (cll_sd) {
         const AVContentLightMetadata *cll =
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index b77da1c2f1..0c884fe04d 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -231,12 +231,13 @@  static int handle_side_data(AVCodecContext *avctx, const x265_api *api,
 {
     const AVFrameSideData *cll_sd =
         av_frame_side_data_get(
-            (const AVFrameSideData **)avctx->frame_side_data,
-            avctx->nb_frame_side_data, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+            (const AVFrameSideData **)avctx->encoder_side_data,
+            avctx->nb_encoder_side_data, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
     const AVFrameSideData *mdcv_sd =
         av_frame_side_data_get(
-            (const AVFrameSideData **)avctx->frame_side_data,
-            avctx->nb_frame_side_data, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+            (const AVFrameSideData **)avctx->encoder_side_data,
+            avctx->nb_encoder_side_data,
+            AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
 
     if (cll_sd) {
         const AVContentLightMetadata *cll =
diff --git a/libavcodec/options.c b/libavcodec/options.c
index 29b961411e..dd5c697da3 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -177,7 +177,7 @@  void avcodec_free_context(AVCodecContext **pavctx)
     av_freep(&avctx->rc_override);
     av_channel_layout_uninit(&avctx->ch_layout);
     av_frame_side_data_free(
-        &avctx->frame_side_data, &avctx->nb_frame_side_data);
+        &avctx->encoder_side_data, &avctx->nb_encoder_side_data);
 
     av_freep(pavctx);
 }