[FFmpeg-devel] [PATCH 1/7] avutil: add an API to handle 3D Reference Displays Information

[Timo Rothenpieler]

On 01.07.2025 22:37, James Almer wrote:
> On 7/1/2025 3:49 PM, Timo Rothenpieler wrote:
>> On 23.06.2025 17:44, Timo Rothenpieler wrote:
>>> On 16.06.2025 20:26, Andreas Rheinhardt wrote:
>>>> Timo Rothenpieler:
>>>>> On 16.06.2025 14:55, James Almer wrote:
>>>>>> On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
>>>>>>> On 13/06/2025 16:07, Timo Rothenpieler wrote:
>>>>>>>> On 10/06/2025 00:09, Andreas Rheinhardt wrote:
>>>>>>>>> James Almer:
>>>>>>>>>> On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
>>>>>>>>>>> On 08.06.2025 17:45, James Almer wrote:
>>>>>>>>>>>> On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
>>>>>>>>>>>>> Timo Rothenpieler:
>>>>>>>>>>>>>> From: James Almer <jamrial at gmail.com>
>>>>>>>>>>>>>>
>>>>>>>>>>>>> I don't like that you add another allocator for this; instead
>>>>>>>>>>>>> we should
>>>>>>>>>>>>> add a generic allocator for the frame side-data types.
>>>>>>>>>>>>
>>>>>>>>>>>> Wont work for packet side data. And i purposely didn't add yet
>>>>>>>>>>>> another allocator that inserts the result into a frame, like
>>>>>>>>>>>> there's
>>>>>>>>>>>> in so many other modules, because eventually the generic one
>>>>>>>>>>>> would be
>>>>>>>>>>>> introduced.
>>>>>>>>>>>>
>>>>>>>>>>>> You said you wanted to take over my work on the generic 
>>>>>>>>>>>> allocator,
>>>>>>>>>>>> but not sure if you did anything with it. The core issue was
>>>>>>>>>>>> handling
>>>>>>>>>>>> more complex types that didn't just have an extra nb_blocks
>>>>>>>>>>>> argument.
>>>>>>>>>>>
>>>>>>>>>>> So, what is the conclusion here?
>>>>>>>>>>> I'd like to push this set if you can come to an agreement.
>>>>>>>>>>>
>>>>>>>>>>> I haven't looked into it much, but implementing av_tdrdi_alloc()
>>>>>>>>>>> in a
>>>>>>>>>>> generic way does seem a bit hacky. And other types might need 
>>>>>>>>>>> even
>>>>>>>>>>> more info for the allocation.
>>>>>>>>>>
>>>>>>>>>> The set LGTM. A custom frame side data allocator does not 
>>>>>>>>>> imply an
>>>>>>>>>> allocator is required for this struct. Frames are not the only 
>>>>>>>>>> user.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I was thinking about a generic side data allocator (for the
>>>>>>>>> AVFrameSideDataType stuff, but it is not meant to be AVFrame
>>>>>>>>> specific),
>>>>>>>>> so that it can be used by more than just AVFrames. Will write one
>>>>>>>>> tomorrow.
>>>>>>>>
>>>>>>>> Would you be okay to for now pushing this with the API turned into
>>>>>>>> an internal one, so the public API can then be decided and
>>>>>>>> implemented later?
>>>>>>>
>>>>>>> Will apply soon with the new public APIs turned into an ff_ symbol.
>>>>>> No, the allocator is required, and an internal function like you
>>>>>> suggest would need a separate internal header, and will make the
>>>>>> struct virtually unusable outside lavu for non frame side data users.
>>>>>
>>>>> It's only meant as a temporary measure until the final public API is
>>>>> figured out, so the rest can move on.
>>>> Sorry for being an asshole and forgetting about this. Will really 
>>>> send a
>>>> patch for what I intend tomorrow.
>>>
>>> ping
>>>
>>> Or did I simply miss the patch?
>>
>> ping
> 
> Please push it. I shouldn't repeat myself in that the struct needs its 
> own allocator regardless of what we do with frame side data.
> 

sure, applied