[FFmpeg-devel] [PATCH 001/279] Add a new channel layout API

[Marton Balint]

On Tue, 14 Dec 2021, James Almer wrote:

> On 12/14/2021 3:54 PM, Nicolas George wrote:
>>  James Almer (12021-12-14):
>>>  We add a const uint8_t* field to AVChannelCustom. If the user wants to
>>>  allocate the strings instead, and not worry about their lifetime, they
>>>  can
>>>  provide the layout with a custom free() function that will take care of
>>>  cleaning anything they came up with on uninit().
>>
>>  I understood what you suggested, and it amounts to letting API users
>>  fend for themselves. We can do that, but I would prefer if we only did
>>  on last resort, if we do not find a more convenient solution.
>
> There's "char name[16]". Bigger than a pointer (Could be 8 bytes instead, but 
> then it's kinda small). The user will not have to worry about the lifetime of 
> anything then.
>
> I'm attaching a diff that goes on top of the last patch of this set 
> implementing this. It also adds support for these custom names to 
> av_channel_layout_describe(), av_channel_layout_index_from_string() and 
> av_channel_layout_channel_from_string(), including tests.

I'd rather not mix custom labels with our fixed names for channels. E.g. 
what if a label conflicts with our existing channel names? If the user 
wants to specify a channel based on its label, that should be a separate 
syntax IMHO.

Overall, having a char name[16] is still kind of limiting, e.g. a label 
read from a muxed file will be truncated, I'd rather not have anything.

Here is one more idea, kind of a mix of what I read so far: Let's refcount 
only the dynamically allocated part of AVChannelLayout. Something like:

typedef struct AVChannelLayout {
     enum AVChannelOrder order;
     int nb_channels;
     uint64_t mask;
     AVBufferRef *custom;
} AVChannelLayout;

And the reference counted data could point to an array of AVChannelCustom 
entries. And AVChannelCustom can have AVDictionary *metadata, because it 
is refcounted.

Regards,
Marton