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

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-02-18 17:52:00)
>> 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.)
> 
> Hmm, then it's not clear to me what from my original email did you
> disagree with. My point was that any changes to fftools can be done
> right now in a very straightforward manner. It will add new compiler
> warnings, but that's a good thing since the current code is already
> suspect and SHOULD trigger warnings.
> 
I do not really disagree with you; I actually wanted to suggest to
bring forward the changes to avformat_open_input() in order not to
trigger new compiler warnings and to allow users to write future-proof
code now.

- Andreas