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

[Zhao Zhili]

> On Nov 2, 2019, at 1:16 AM, Marton Balint <cus at passwd.hu> wrote:
> 
> 
> 
> 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.

APIchanges says (if it counts as part of the documentation):

2014-02-24 - 3e1f241 / d161ae0 - lavu 52.69.100 / 53.8.0 - frame.h
  Add av_frame_remove_side_data() for removing a single side data
  instance from a frame.

> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org <mailto:ffmpeg-devel-request at ffmpeg.org> with subject "unsubscribe".