[FFmpeg-devel] [PATCH 1/6] avformat/format: add av_demuxer_find_by_ext

[Gyan Doshi]

On 18-02-2020 10:22 pm, Andreas Rheinhardt wrote:
> Anton Khirnov:
>> Quoting Andreas Rheinhardt (2020-02-18 16:01:00)
>>> Anton Khirnov:
>>>> Quoting Gyan Doshi (2020-02-01 06:15:30)
>>>>>
>>>>> On 31-01-2020 10:41 pm, Andreas Rheinhardt wrote:
>>>>>> Gyan Doshi:
>>>>>>> Allows selecting demuxer by extension which are more widely recognized
>>>>>>> by users.
>>>>>>>
>>>>>>> Conditional cast added since this function will usually be called after
>>>>>>> av_find_input_format, and so matches its return type.
>>>>>> That's not a good point. av_demuxer_find_by_ext() already always
>>>>>> returns const AVInputFormat *, so you casting the const away when
>>>>>> returning is pointless. Furthermore, any caller that wants to use this
>>>>>> new function can simply use a pointer to const AVInputFormat to work
>>>>>> with both av_find_input_format() and av_demuxer_find_by_ext(). And
>>>>>> after all, adding const makes the code more future-proof
>>>>>> (av_find_input_format() will return const AVInputFormat * after the
>>>>>> next major bump).
>>>>> Ok, I don't think I should add const to the pointers at the receiving
>>>>> end (fftools) since they are global variables and may not be acceptable
>>>>> as const. So I'll cast away the const when receiving and remove the
>>>>> conditional cast.
>>>> Why not? Nothing can conceivably modify the content of those structs, so
>>>> simply adding const to all their uses should work.
>>>>
>>> Then avformat_open_input() would have to be modified to accept a const
>>> AVInputFormat *, otherwise one would get compiler warnings. And then
>>> one would have to cast the const away in avformat_open_input() (should
>>> probably already have been made in 3aa6208d to allow users to write
>>> future-proof-code).
>> It's already been modified to be const on the next bump, so the warning
>> would only be temporary. And we are planning to do a bump pretty soon
>> IIUC.
> The intention is to allow users to write future-proof code now. And
> also to test whether further changes are necessary (e.g. to the
> av_opt-API).
>> And I'm not sure if changing a function parameter from pointer to
>> pointer-to-const is actually an API break. It's merely an extra
>> constraint on what a function is allowed to do.
>>
> Adding const to avformat_open_input() would be both API- as well as
> ABI-compatible (that's also why I said that it should already have
> been done in 3aa6208d).
>
>> And besides, those structs are actually constant. They must not be
>> modified from the outside. So compiler warnings would be entirely
>> appropriate to remind us about invalid code.
>>
> I agree. (E.g. I have proposed [1] to make avio_enum_protocols()
> const-correct.)

Since the major bump is meant to be soon, I'll just wait till then.

Gyan