Re: [FFmpeg-devel] [PATCH v10 00/14] encoder AVCodecContext configuration side data

["Jan Ekström" <jeebjp@gmail.com>]

On Wed, Mar 20, 2024 at 7:00 PM James Almer <jamrial@gmail.com> wrote:
>
> 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.

Thanks.

Applied everything except for "avutil/frame: add helper for adding
side data w/ AVBufferRef to array".

Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".