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

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Tue Feb 18 18:52:00 EET 2020


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.)

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-August/248675.html


More information about the ffmpeg-devel mailing list