Re: [FFmpeg-devel] [FFmpeg-cvslog] ffmpeg: pass first video AVFrame's side data to encoder

Re: [FFmpeg-devel] [FFmpeg-cvslog] ffmpeg: pass first video AVFrame's side data to encoder

[Anton Khirnov]

Quoting Jan Ekström (2024-03-20 18:31:49)
> ffmpeg | branch: master | Jan Ekström <jeebjp@gmail.com> | Mon Jan  9 20:52:30 2023 +0200| [8f4b173029aa9ecefb1370c954ef769664920d08] | committer: Jan Ekström
> 
> ffmpeg: pass first video AVFrame's side data to encoder
> 
> This enables further configuration of output based on the results
> of input decoding and filtering in a similar manner as the color
> information.
> 
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8f4b173029aa9ecefb1370c954ef769664920d08
> ---
> 
>  fftools/ffmpeg_enc.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
> index c9a12af139..f01be1c22f 100644
> --- a/fftools/ffmpeg_enc.c
> +++ b/fftools/ffmpeg_enc.c
> @@ -246,6 +246,21 @@ int enc_open(void *opaque, const AVFrame *frame)
>          enc_ctx->colorspace             = frame->colorspace;
>          enc_ctx->chroma_sample_location = frame->chroma_location;
>  
> +        for (int i = 0; i < frame->nb_side_data; i++) {
> +            ret = av_frame_side_data_clone(
> +                &enc_ctx->decoded_side_data, &enc_ctx->nb_decoded_side_data,
> +                frame->side_data[i], AV_FRAME_SIDE_DATA_FLAG_UNIQUE);

I objected to this indiscriminate copying in a previous review.

There is no reason to expect side data to apply to the whole stream just
because it happens to apply to the first frame. For many side data types
it does not even make sense, as they are fundamentally per-frame.

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

Re: [FFmpeg-devel] [FFmpeg-cvslog] ffmpeg: pass first video AVFrame's side data to encoder

[James Almer]

On 3/22/2024 6:52 AM, Anton Khirnov wrote:
> Quoting Jan Ekström (2024-03-20 18:31:49)
>> ffmpeg | branch: master | Jan Ekström <jeebjp@gmail.com> | Mon Jan  9 20:52:30 2023 +0200| [8f4b173029aa9ecefb1370c954ef769664920d08] | committer: Jan Ekström
>>
>> ffmpeg: pass first video AVFrame's side data to encoder
>>
>> This enables further configuration of output based on the results
>> of input decoding and filtering in a similar manner as the color
>> information.
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8f4b173029aa9ecefb1370c954ef769664920d08
>> ---
>>
>>   fftools/ffmpeg_enc.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
>> index c9a12af139..f01be1c22f 100644
>> --- a/fftools/ffmpeg_enc.c
>> +++ b/fftools/ffmpeg_enc.c
>> @@ -246,6 +246,21 @@ int enc_open(void *opaque, const AVFrame *frame)
>>           enc_ctx->colorspace             = frame->colorspace;
>>           enc_ctx->chroma_sample_location = frame->chroma_location;
>>   
>> +        for (int i = 0; i < frame->nb_side_data; i++) {
>> +            ret = av_frame_side_data_clone(
>> +                &enc_ctx->decoded_side_data, &enc_ctx->nb_decoded_side_data,
>> +                frame->side_data[i], AV_FRAME_SIDE_DATA_FLAG_UNIQUE);
> 
> I objected to this indiscriminate copying in a previous review.
> 
> There is no reason to expect side data to apply to the whole stream just
> because it happens to apply to the first frame. For many side data types
> it does not even make sense, as they are fundamentally per-frame.

So this would need to be handled by having decoders export global side 
data in decoded_side_data directly, so there's no need to take them from 
frames here (which would include per frame side data, like hdr10 and 
closed captions), right?
_______________________________________________
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".

Re: [FFmpeg-devel] [FFmpeg-cvslog] ffmpeg: pass first video AVFrame's side data to encoder

[Anton Khirnov]

Quoting James Almer (2024-03-22 20:38:38)
> On 3/22/2024 6:52 AM, Anton Khirnov wrote:
> > I objected to this indiscriminate copying in a previous review.
> > 
> > There is no reason to expect side data to apply to the whole stream just
> > because it happens to apply to the first frame. For many side data types
> > it does not even make sense, as they are fundamentally per-frame.
> 
> So this would need to be handled by having decoders export global side 
> data in decoded_side_data directly, so there's no need to take them from 
> frames here (which would include per frame side data, like hdr10 and 
> closed captions), right?

That's not enough, you still need to propagate this data through the
filtergraph.

I'm leaning towards something like a side data descriptor, that would
tell callers (among other things) whether a side data type is meaningful
in global headers.

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