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

[James Almer <jamrial@gmail.com>]

On 12/8/2023 8:59 AM, Jan Ekström wrote:
> On Sun, Nov 26, 2023 at 9:58 PM Jan Ekström <jeebjp@gmail.com> wrote:
>>
>> Differences to v3:
>> 1. rebased on top of current master
>> 2. moved the addition of multiple side data entries from a generic
>>     av_frame_side_data_set_extend to avcodec as per request from James.
>> 4. adopted various things noted by reviews
>>
>> Comparison URL (mostly configure and wrappers, avutil/frame.c):
>> https://github.com/jeeb/ffmpeg/compare/avcodec_cll_mdcv_side_data_v4..avcodec_cll_mdcv_side_data_v5
>>
>> 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
> 
> Ping.
> 
> I'd like to understand whether:
> 
> * people are fine with the struct which lets you pass things as a
> single argument, or they would like to move all the helper functions
> to counter and pointer.

IMO we should do the same we did for packet side data, so IN/OUT pointer 
and size as arguments. It should also match the function signatures as 
much as possible. So for example:

> AVFrameSideData *av_frame_side_data_new(AVPacketSideData ***sd, int *nb_sd,
>                                         enum AVFrameSideDataType type,
>                                         size_t size, int flags);

> AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
>                                         enum AVFrameSideDataType type,
>                                         void *data, size_t size, int flags);

> AVFrameSideData *av_packet_side_data_add_from_buf(AVFrameSideData ***sd, int *nb_sd,
>                                                   enum AVFrameSideDataType type,
>      /* making a new ref, not taking ownership */ AVBufferRef *buf, int flags);

> const AVFrameSideData *av_frame_side_data_get(const AVFrameSideData **sd,
>                                               int nb_sd,
>                                               enum AVFrameSideDataType type);

> void av_frame_side_data_remove(AVFrameSideData ***sd, int *nb_sd,
>                                enum AVFrameSideDataType type);

> void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);

> const char *av_frame_side_data_name(enum AVFrameSideDataType type);

And since unlike with packet we allow more than one entry per frame side 
data type, you could add something like:

> const AVFrameSideData *av_frame_side_data_iterate(const AVFrameSideData **sd,
>                                                   int nb_sd,
>                                                   enum AVPacketSideDataType type,
>                                                   void **opaque);

> * should the avcodec helper take in the struct/{counter,pointer}, or
> should it instead take in a const AVFrame pointer?

The former. There already are helpers for side data in AVFrame, and the 
point of this set is to add such side data to AVCodecContext.

> 
> as I'd like to start pulling this in, and move towards working on
> implemented side data listing in AVCodecs, as well as adding generic
> implementations for specific codecs (such as H.264/HEVC/AV1 having CBS
> create side data generically, without the need of specific AVCodecs
> implementing things),
> 
> 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".
_______________________________________________
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".