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.
_______________________________________________ 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".
