diff mbox series

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

Message ID 20200131142608.42729-1-ffmpeg@gyani.pro
State New
Headers show
Series [FFmpeg-devel,1/6] avformat/format: add av_demuxer_find_by_ext
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Gyan Doshi Jan. 31, 2020, 2:26 p.m. UTC
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.
---
 libavformat/avformat.h |  5 +++++
 libavformat/format.c   | 15 +++++++++++++++
 libavformat/version.h  |  2 +-
 3 files changed, 21 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Jan. 31, 2020, 5:11 p.m. UTC | #1
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).

> ---
>  libavformat/avformat.h |  5 +++++
>  libavformat/format.c   | 15 +++++++++++++++
>  libavformat/version.h  |  2 +-
>  3 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 9b9b634ec3..9172ddbc8a 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -2237,6 +2237,11 @@ int avformat_alloc_output_context2(AVFormatContext **ctx, ff_const59 AVOutputFor
>   */
>  ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
>  
> +/**
> + * Find AVInputFormat based on an extension.
> + */
> +const AVInputFormat *av_demuxer_find_by_ext(const char *extension);
> +
>  /**
>   * Guess the file format.
>   *
> diff --git a/libavformat/format.c b/libavformat/format.c
> index c47490c8eb..9dda6df676 100644
> --- a/libavformat/format.c
> +++ b/libavformat/format.c
> @@ -125,6 +125,21 @@ ff_const59 AVInputFormat *av_find_input_format(const char *short_name)
>      return NULL;
>  }
>  
> +const AVInputFormat *av_demuxer_find_by_ext(const char *extension)
> +{
> +    const AVInputFormat *fmt = NULL;
> +    void *i = 0;

Use NULL, it's a pointer after all (yes, I know that
av_demuxer_iterate() treats it as a uintptr_t internally).

> +    while ((fmt = av_demuxer_iterate(&i)))
> +        if (fmt->extensions && av_match_name(extension, fmt->extensions))
> +#if FF_API_AVIOFORMAT
> +            return (AVInputFormat*)fmt;
> +#else
> +            return fmt;
> +#endif

As has been said: The first branch of the #if should be deleted (as
the #if, of course).

- Andreas
Gyan Doshi Feb. 1, 2020, 5:15 a.m. UTC | #2
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.

Sounds fine?

Gyan

>
>> ---
>>   libavformat/avformat.h |  5 +++++
>>   libavformat/format.c   | 15 +++++++++++++++
>>   libavformat/version.h  |  2 +-
>>   3 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 9b9b634ec3..9172ddbc8a 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -2237,6 +2237,11 @@ int avformat_alloc_output_context2(AVFormatContext **ctx, ff_const59 AVOutputFor
>>    */
>>   ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
>>   
>> +/**
>> + * Find AVInputFormat based on an extension.
>> + */
>> +const AVInputFormat *av_demuxer_find_by_ext(const char *extension);
>> +
>>   /**
>>    * Guess the file format.
>>    *
>> diff --git a/libavformat/format.c b/libavformat/format.c
>> index c47490c8eb..9dda6df676 100644
>> --- a/libavformat/format.c
>> +++ b/libavformat/format.c
>> @@ -125,6 +125,21 @@ ff_const59 AVInputFormat *av_find_input_format(const char *short_name)
>>       return NULL;
>>   }
>>   
>> +const AVInputFormat *av_demuxer_find_by_ext(const char *extension)
>> +{
>> +    const AVInputFormat *fmt = NULL;
>> +    void *i = 0;
> Use NULL, it's a pointer after all (yes, I know that
> av_demuxer_iterate() treats it as a uintptr_t internally).
>
>> +    while ((fmt = av_demuxer_iterate(&i)))
>> +        if (fmt->extensions && av_match_name(extension, fmt->extensions))
>> +#if FF_API_AVIOFORMAT
>> +            return (AVInputFormat*)fmt;
>> +#else
>> +            return fmt;
>> +#endif
> As has been said: The first branch of the #if should be deleted (as
> the #if, of course).
>
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Gyan Doshi Feb. 12, 2020, 5:42 p.m. UTC | #3
On 01-02-2020 10:45 am, Gyan Doshi wrote:
>
>
> 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.
>
> Sounds fine?

Ping.

Gyan
Gyan Doshi Feb. 15, 2020, 2:38 p.m. UTC | #4
On 12-02-2020 11:12 pm, Gyan Doshi wrote:
>
>
> On 01-02-2020 10:45 am, Gyan Doshi wrote:
>>
>>
>> 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.
>>
>> Sounds fine?
>
> Ping.

Ping x2.
Andreas Rheinhardt Feb. 16, 2020, 11:49 a.m. UTC | #5
Gyan Doshi:
> 
> 
> 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.
> 
> Sounds fine?
> 

Given that a user would have to cast the const away for
avformat_open_input() if he worked with pointers to const
AVInputFormat I don't want to block your patch because of
const-correctness. So proceed as you wish.

- Andreas
Anton Khirnov Feb. 18, 2020, 2:28 p.m. UTC | #6
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.
Andreas Rheinhardt Feb. 18, 2020, 3:01 p.m. UTC | #7
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).
But the av_opt-API seems to be based around non-const structures which
may be a problem with the av_opt_find()-calls in open_input_file() in
ffmpeg_opt.c.

- Andreas
Anton Khirnov Feb. 18, 2020, 4:26 p.m. UTC | #8
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.

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.

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.
Andreas Rheinhardt Feb. 18, 2020, 4:52 p.m. UTC | #9
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
Gyan Doshi Feb. 18, 2020, 4:54 p.m. UTC | #10
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
Anton Khirnov Feb. 18, 2020, 4:59 p.m. UTC | #11
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.
Andreas Rheinhardt Feb. 18, 2020, 5:05 p.m. UTC | #12
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
diff mbox series

Patch

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 9b9b634ec3..9172ddbc8a 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -2237,6 +2237,11 @@  int avformat_alloc_output_context2(AVFormatContext **ctx, ff_const59 AVOutputFor
  */
 ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
 
+/**
+ * Find AVInputFormat based on an extension.
+ */
+const AVInputFormat *av_demuxer_find_by_ext(const char *extension);
+
 /**
  * Guess the file format.
  *
diff --git a/libavformat/format.c b/libavformat/format.c
index c47490c8eb..9dda6df676 100644
--- a/libavformat/format.c
+++ b/libavformat/format.c
@@ -125,6 +125,21 @@  ff_const59 AVInputFormat *av_find_input_format(const char *short_name)
     return NULL;
 }
 
+const AVInputFormat *av_demuxer_find_by_ext(const char *extension)
+{
+    const AVInputFormat *fmt = NULL;
+    void *i = 0;
+    while ((fmt = av_demuxer_iterate(&i)))
+        if (fmt->extensions && av_match_name(extension, fmt->extensions))
+#if FF_API_AVIOFORMAT
+            return (AVInputFormat*)fmt;
+#else
+            return fmt;
+#endif
+
+    return NULL;
+}
+
 ff_const59 AVInputFormat *av_probe_input_format3(ff_const59 AVProbeData *pd, int is_opened,
                                       int *score_ret)
 {
diff --git a/libavformat/version.h b/libavformat/version.h
index 15fdb73e3e..be22abc010 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  58
-#define LIBAVFORMAT_VERSION_MINOR  37
+#define LIBAVFORMAT_VERSION_MINOR  38
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \