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

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

-- 
Anton Khirnov