[FFmpeg-devel] [PATCH v2 1/4] frame: handle add side data with the same type

[Marton Balint]

On Fri, 1 Nov 2019, "zhilizhao(赵志立)" wrote:

>
>
>> On Nov 1, 2019, at 8:13 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>> 
>> On Fri, Nov 1, 2019 at 1:03 PM <quinkblack at foxmail.com> wrote:
>>> 
>>> From: Zhao Zhili <zhilizhao at tencent.com>
>>> 
>>> ---
>>> libavutil/frame.c | 13 +++++++++++++
>>> libavutil/frame.h |  4 ++++
>>> 2 files changed, 17 insertions(+)
>>> 
>> 
>> I believe there have been some use-cases, especially around
>> closed-captions, where multiple blocks of the same type have been
>> used, somehow. Since this is really an API change, not sure its such a
>> good idea.
>
> I guess it may be too late to change the behavior.

I am not sure, all our API around side data (get/remove) is based on the 
assumption that a single entry of a type exists. Also for packet/stream 
side data it is already assumed as far as I see. So at least for the sake 
of consistency it should be the same way. Maybe we should print a 
deprecation warning if something adds multiple side data of the same type. 
And later sometime it can be changed to actually replace the old side 
data.

> Whether keep the current
> behavior or not, remove_side_data is broken. It can remove multiple side data
> and miss a few.

Yes.

>
> It can be fixed in two different ways:
>
> 1. Remove the first side data of the supplied type, like get_side_data().
> 2. Remove all side data of the supplied type.
>
> The first solution match current documentation.

I don't see why the 2nd would contradict docs:

/**
  * If side data of the supplied type exists in the frame, free it and remove it
  * from the frame.
  */

So I'd vote for 2), which removes all side data of the specified type.

Regards,
Marton