[FFmpeg-devel] [PATCH v4 03/21] cbs: Describe allocate/free methods in tabular form

[Mark Thompson]

On 24/02/2020 22:42, Andreas Rheinhardt wrote:
> Mark Thompson:
>> On 24/02/2020 22:10, Andreas Rheinhardt wrote:
>>> Mark Thompson:
>>>> On 24/02/2020 17:19, Andreas Rheinhardt wrote:
>>>>> Mark Thompson:
>>>>>> Unit types are split into three categories, depending on how their
>>>>>> content is managed:
>>>>>> * POD structure - these require no special treatment.
>>>>>> * Structure containing references to refcounted buffers - these can use
>>>>>>   a common free function when the offsets of all the internal references
>>>>>>   are known.
>>>>>> * More complex structures - these still require ad-hoc treatment.
>>>>>>
>>>>>> For each codec we can then maintain a table of descriptors for each set of
>>>>>> equivalent unit types, defining the mechanism needed to allocate/free that
>>>>>> unit content.  This is not required to be used immediately - a new alloc
>>>>>> function supports this, but does not replace the old one which works without
>>>>>> referring to these tables.
>>>>>> ---
>>>>>>  libavcodec/cbs.c          | 69 +++++++++++++++++++++++++++++++++++++++
>>>>>>  libavcodec/cbs.h          |  9 +++++
>>>>>>  libavcodec/cbs_internal.h | 60 ++++++++++++++++++++++++++++++++++
>>>>>>  3 files changed, 138 insertions(+)
>>>>>>
>>>>>> ...
>>>>>> +typedef struct CodedBitstreamUnitTypeDescriptor {
>>>>>> ...
>>>>>> +} CodedBitstreamUnitTypeDescriptor;
>>>>>
>>>>> I wonder whether you should add const to the typedef in order to omit
>>>>> it everywhere else. After all, no CodedBitstreamUnitTypeDescriptor
>>>>> will ever be assembled during runtime.
>>>>
>>>> It definitely makes sense to add it to reduce errors.  Not so sure about the removing it from everywhere else - the fact that it looks wrong at the point of use probably causes more confusion.
>>>>
>>>> So, I've done the first part but not the second (helpfully, redundant type qualifiers have no effect).
>>>
>>> MSVC emits a warning (or just a note or so) for this.
>> Urgh.  Is that definitely intended or is it a bug in the compiler?  The C standard is very clear that this is fine (C11 6.7.3).
>>
> This is also the way it is in C99, but given that [1] says that it
> leads to an error with MSVC in ANSI-C mode (which means C90), I looked
> at C90 and found:
> "The same type qualifier shall not appear more than once in the same
> specifier list or qualifier list, either directly or via one or more
> typedefs."

Ok, that's fatal to this plan.  I think we're better with the consts on the individual cases (less confusing, if slightly less efficient), so I've reverted back to the original.

Thanks,

- Mark