[FFmpeg-devel] [PATCH 6/7] avcodec: Make avcodec_decoder_subtitles2 accept a const AVPacket*

[Hendrik Leppkes]

On Thu, Apr 14, 2022 at 9:33 PM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2022-03-31 00:49:57)
> >> From: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> >> ---
> >>  doc/APIchanges            | 3 +++
> >>  fftools/ffmpeg.c          | 4 ++--
> >>  fftools/ffprobe.c         | 2 +-
> >>  libavcodec/avcodec.h      | 3 +--
> >>  libavcodec/decode.c       | 9 ++++-----
> >>  libavcodec/version.h      | 2 +-
> >>  tools/target_dec_fuzzer.c | 4 ++--
> >>  7 files changed, 14 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index 1a9f0a303e..326a3c721c 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
> >>
> >>  API changes, most recent first:
> >>
> >> +2022-03-30 - xxxxxxxxxx - lavc 59.26.100 - avcodec.h
> >> +  avcodec_decode_subtitle2() now accepts const AVPacket*.
> >
> > I vaguely recall C++ having a problem with such changes. Anybody
> > remembers the details? Do we care?
> >
>
> You did something like this in 0a0f19b577d54ff2e72cc9a0fe027952db83f332
> without a major bump; IIRC there are more instances likes this.
>
> For C functions, the signature is not part of the exported symbol name
> (as zhilizhao already said) and the ABIs just presume that the caller
> and callee agree on the signature. Constifying this parameter does not
> break this, as "pointers to qualified or unqualified versions of
> compatible types shall have the same representation and alignment
> requirements" (C11 6.2.5/26).
>
> Ordinary API usage will be fine, because the conversion from AVPacket*
> to const AVPacket* is safe and therefore performed automatically; it
> requires no cast. API-wise there is only one thing that might cause
> problems: If one does not use avcodec_decode_subtitle2 directly, but
> via a function pointer like this:
>
> #include <libavcodec/avcodec.h>
>
> typedef int (*unconst)(AVCodecContext *, AVSubtitle *, int *, AVPacket *);
>
> void foo(void)
> {
>     unconst funcptr = avcodec_decode_subtitle2;
>
>     /* use funcptr */
> }
>
> Here the initialization of avcodec_decode_subtitle2 is not fine; for C
> compilers will by default emit a warning, for C++ it is common to fail.
> But avcodec_decode_subtitle2 is the only function with this signature
> at all, so it makes no sense to use function pointers instead of using
> this function directly. I don't think that anyone does and therefore my
> answer to your last question is: I don't really care given that ordinary
> usages are fine.
>

If one does manual dynamic loading (eg. for optional components), you
would use a function pointer .. for all functions, not just this one.
On the other hand, if you use C++ and you manually define the type for
every function pointer (as we don't define function pointer types),
its a rather fragile construct to begin with, especially as you could
just infer the types automatically using decltype constructs, in which
case there would be no issues as it just updates automatically.

In theory such a change can break someones code, but.. its not great
code to begin with and requires constant maintenance.

- Hendrik