[FFmpeg-devel] [PATCH 2/4] libavutil: add side data AV_FRAME_DATA_BOUNDING_BOXES

[Lynne]

Feb 20, 2021, 15:13 by yejun.guo at intel.com:

>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Lynne
>> Sent: 2021年2月20日 6:54
>> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH 2/4] libavutil: add side data
>> AV_FRAME_DATA_BOUNDING_BOXES
>>
>> Feb 19, 2021, 07:59 by yejun.guo at intel.com:
>>
>> > Signed-off-by: Guo, Yejun <yejun.guo at intel.com>
>> > ---
>> >  doc/APIchanges      | 2 ++
>> >  libavutil/frame.c   | 1 +
>> >  libavutil/frame.h   | 7 +++++++
>> >  libavutil/version.h | 2 +-
>> >  4 files changed, 11 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/doc/APIchanges b/doc/APIchanges
>> > index c353d2d281..3c6e9e351e 100644
>> > --- a/doc/APIchanges
>> > +++ b/doc/APIchanges
>> > @@ -14,6 +14,8 @@ libavutil:     2017-10-21
>> >
>>
>> This won't really work.
>> Side data must be public but the header it's specified in is neither installed,
>> neither in lavu. There's no such thing as ffmpeg-only private data.
>>
>> If we do plan to make this side data public, it'll require a lot more
>> considerations,
>> and versioning like with the film grain side data, since as you pointed out it
>> may
>> be used by codecs.
>>
>
> Thanks for your feedback.
>
> Could you share a bit more about the 'versioning' of film grain? thanks.
>
> I checked file film_grain_params.h/c and do not find it is version relative.
>
> As for the bounding box, the BoundingBoxHeader.bbox_size acts like
> a version checker. (different bbox_size if the struct is changed later)
>

That's not really a good solution. I meant having a dedicated and documented
version field which switches structs.
Changing the struct size means we can only add fields, so if we need to alter
the behavior of previous fields we'll end up with a data format more convoluted
than mpeg audio codecs.


>> For now, although it's somewhat less appropriate, you can replace
>> the side data with frame->private_ref. It's an AVBufferRef that's for
>> use by a single internal library, and we don't use it anywhere except
>> in lavc for now.
>>
>
> Thanks for pointing this possible way, I'll investigate it.
> I would prefer side data which is possible the final solution.
>

I'd rather not commit us to a side data format that may change
in the future and has no lavfi-external use. Could you implement it as a
private_ref, at least for the time being? It's lavfi-only, so it makes sense
for now, plus we can keep changing it, and once an external user
appears we can make it proper public side data?