[FFmpeg-devel] [PATCH v10 00/14] encoder AVCodecContext configuration side data
James Almer
jamrial at gmail.com
Wed Mar 20 19:00:36 EET 2024
On 3/18/2024 6:31 PM, Jan Ekström wrote:
> Differences to v9:
> 1. rebased on top of current master
> 2. renamed the avctx AVFrameSideData array according to Anton's naming sense,
> as at this point that is now OK by James as well and I just want to get this
> done with.
> 3. removed the avctx helper to get through review.
> 4. added "Afterwards owned and freed by the encoder" comment into the
> description of the side data array.
> 5. added APIchanges (missing the AVBufferRef function since that is for now to
> be skipped) and avutil/avcodec bumps.
> 6. removed all references to "_set" from internal functions and the newly added
> test. The only thing that is left around that is a set is a 100% internal
> helper struct for the side_data_array test called FrameSideDataSet.
>
> Comparison URL (mostly configure and wrappers, avutil/frame.c):
> https://github.com/jeeb/ffmpeg/compare/avcodec_cll_mdcv_side_data_v9..avcodec_cll_mdcv_side_data_v10
>
> 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
> {avutil/version,APIchanges}: bump, document new AVFrameSideData
> functions
> avcodec: add frame side data array to AVCodecContext
> 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 +
> doc/APIchanges | 8 ++
> fftools/ffmpeg_enc.c | 15 +++
> libavcodec/avcodec.h | 13 +++
> libavcodec/libsvtav1.c | 69 ++++++++++++
> libavcodec/libx264.c | 80 +++++++++++++
> libavcodec/libx265.c | 89 +++++++++++++++
> libavcodec/options.c | 2 +
> libavcodec/version.h | 4 +-
> libavutil/Makefile | 1 +
> libavutil/frame.c | 180 +++++++++++++++++++++++++-----
> libavutil/frame.h | 88 +++++++++++++++
> libavutil/tests/side_data_array.c | 103 +++++++++++++++++
> libavutil/version.h | 2 +-
> 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_array | 14 +++
> 20 files changed, 701 insertions(+), 33 deletions(-)
> create mode 100644 libavutil/tests/side_data_array.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_array
LGTM. Except for patch 7 which is still in discussion and is currently
not required, the rest can go in.
More information about the ffmpeg-devel
mailing list