diff mbox series

[FFmpeg-devel,v7,09/14] avcodec: add frame side data array to AVCodecContext

Message ID 20240229164307.3535613-10-jeebjp@gmail.com
State New
Headers show
Series [FFmpeg-devel,v7,01/14] avutil/frame: split side data list wiping out to non-AVFrame function | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Jan Ekström Feb. 29, 2024, 4:42 p.m. UTC 
This allows configuring an encoder by using AVFrameSideData.
---
 libavcodec/avcodec.h | 8 ++++++++
 libavcodec/options.c | 2 ++
 2 files changed, 10 insertions(+)

Comments

James Almer March 1, 2024, 4:03 p.m. UTC | #1 
On 2/29/2024 1:42 PM, Jan Ekström wrote:
> This allows configuring an encoder by using AVFrameSideData.

Maybe mention that in the doxy for the field. Explain that it needs to 
be set before avcodec_open2() and is used to initialize an encoder.

> ---
>   libavcodec/avcodec.h | 8 ++++++++
>   libavcodec/options.c | 2 ++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 43859251cc..411f4caad3 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2120,6 +2120,14 @@ typedef struct AVCodecContext {
>        *   an error.
>        */
>       int64_t frame_num;
> +
> +    /**
> +     * Set containing static side data, such as HDR10 CLL / MDCV structures.
> +     * - encoding: set by user
> +     * - decoding: unused
> +     */
> +    AVFrameSideData  **frame_side_data;
> +    int             nb_frame_side_data;
>   } AVCodecContext;
>   
>   /**
> diff --git a/libavcodec/options.c b/libavcodec/options.c
> index 928e430ce9..7e39b49b7e 100644
> --- a/libavcodec/options.c
> +++ b/libavcodec/options.c
> @@ -181,6 +181,8 @@ void avcodec_free_context(AVCodecContext **pavctx)
>       av_freep(&avctx->inter_matrix);
>       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);
>   
>       av_freep(pavctx);
>   }
James Almer March 1, 2024, 4:10 p.m. UTC | #2 
On 3/1/2024 1:03 PM, James Almer wrote:
> On 2/29/2024 1:42 PM, Jan Ekström wrote:
>> This allows configuring an encoder by using AVFrameSideData.
> 
> Maybe mention that in the doxy for the field. Explain that it needs to 
> be set before avcodec_open2() and is used to initialize an encoder.

And that it must be allocated with av_frame_side_data_new(), 
av_frame_side_data_from_sd() or av_frame_side_data_from_buf().

> 
>> ---
>>   libavcodec/avcodec.h | 8 ++++++++
>>   libavcodec/options.c | 2 ++
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 43859251cc..411f4caad3 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -2120,6 +2120,14 @@ typedef struct AVCodecContext {
>>        *   an error.
>>        */
>>       int64_t frame_num;
>> +
>> +    /**
>> +     * Set containing static side data, such as HDR10 CLL / MDCV 
>> structures.
>> +     * - encoding: set by user
>> +     * - decoding: unused
>> +     */
>> +    AVFrameSideData  **frame_side_data;
>> +    int             nb_frame_side_data;
>>   } AVCodecContext;
>>   /**
>> diff --git a/libavcodec/options.c b/libavcodec/options.c
>> index 928e430ce9..7e39b49b7e 100644
>> --- a/libavcodec/options.c
>> +++ b/libavcodec/options.c
>> @@ -181,6 +181,8 @@ void avcodec_free_context(AVCodecContext **pavctx)
>>       av_freep(&avctx->inter_matrix);
>>       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);
>>       av_freep(pavctx);
>>   }
Anton Khirnov March 1, 2024, 6:23 p.m. UTC | #3 
Quoting Jan Ekström (2024-02-29 17:42:56)
> This allows configuring an encoder by using AVFrameSideData.
> ---
>  libavcodec/avcodec.h | 8 ++++++++
>  libavcodec/options.c | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 43859251cc..411f4caad3 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2120,6 +2120,14 @@ typedef struct AVCodecContext {
>       *   an error.
>       */
>      int64_t frame_num;
> +
> +    /**
> +     * Set containing static side data, such as HDR10 CLL / MDCV structures.
> +     * - encoding: set by user

May be set by the caller before avcodec_open2(). Afterwards owned and
freed by the encoder.

> +     * - decoding: unused
> +     */
> +    AVFrameSideData  **frame_side_data;
> +    int             nb_frame_side_data;

I don't like calling the field 'frame' side data, because it is not
associated with any frame.

I'd prefer to call it 'decoded_side_data' or 'raw_side_data', to be
consistent with existing 'coded_side_data'.
diff mbox series

Patch

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 43859251cc..411f4caad3 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2120,6 +2120,14 @@  typedef struct AVCodecContext {
      *   an error.
      */
     int64_t frame_num;
+
+    /**
+     * Set containing static side data, such as HDR10 CLL / MDCV structures.
+     * - encoding: set by user
+     * - decoding: unused
+     */
+    AVFrameSideData  **frame_side_data;
+    int             nb_frame_side_data;
 } AVCodecContext;
 
 /**
diff --git a/libavcodec/options.c b/libavcodec/options.c
index 928e430ce9..7e39b49b7e 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -181,6 +181,8 @@  void avcodec_free_context(AVCodecContext **pavctx)
     av_freep(&avctx->inter_matrix);
     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);
 
     av_freep(pavctx);
 }