[FFmpeg-devel] [PATCH 1/3] lavformat: Prepare to make avio_enum_protocols const correct

[Fu, Linjie]

> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Friday, November 22, 2019 18:02
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/3] lavformat: Prepare to make
> avio_enum_protocols const correct
> 
> Andreas Rheinhardt:
> > Andreas Rheinhardt:
> >> Using avio_enum_protocols works as follows: One initializes a pointer to
> >> void and gives avio_enum_protocols the address of said pointer as
> >> argument; the pointer will be updated to point to a member of the
> >> url_protocols array. Now the address of the pointer can be reused for
> >> another call to avio_enum_protocols.
> >> Said array consists of constant pointers (to constant URLProtocols),
> >> but the user now has a pointer to non-const to it; of course it was always
> >> intended that the user is not allowed to modify what the pointer points
> >> to and this has been enforced by hiding the real type of the underlying
> >> object. But it is better to use a const void ** as parameter to enforce
> >> this. This way avio_enum_protocols can be implemented without
> resorting
> >> to casting a const away or ignoring constness as is done currently.
> >>
> >> Given that this amounts to an ABI and API break, this can only be done
> >> at the next major version bump; as usual, the break is currently hidden
> >> behind an appropriate #if.
> >>
> >> It was also necessary to change the type of a pointer used in
> >> avio_enum_protocols. This makes the line that is not const correct move
> >> as long as the old function signature is used. With the new signature,
> >> avio_enum_protocols will be const correct.
> >>
> >> This change will eventually force changes in their callers, e.g. to
> >> show_protocols in fftools/cmdutils. (This function contains all currently
> >> existing calls to avio_enum_protocols in FFmpeg's codebase. It hasn't
> >> been touched in this commit.)
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> >> ---
> >>  libavformat/avio.h      | 4 ++++
> >>  libavformat/protocols.c | 6 +++++-
> >>  libavformat/version.h   | 3 +++
> >>  3 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/avio.h b/libavformat/avio.h
> >> index 9141642e75..e067ea8985 100644
> >> --- a/libavformat/avio.h
> >> +++ b/libavformat/avio.h
> >> @@ -805,7 +805,11 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t
> **pbuffer);
> >>   *
> >>   * @return A static string containing the name of current protocol or NULL
> >>   */
> >> +#if FF_API_NONCONST_ENUM_PROTOCOLS
> >>  const char *avio_enum_protocols(void **opaque, int output);
> >> +#else
> >> +const char *avio_enum_protocols(const void **opaque, int output);
> >> +#endif
> >>
> >>  /**
> >>   * Pause and resume playing - only meaningful if using a network
> streaming
> >> diff --git a/libavformat/protocols.c b/libavformat/protocols.c
> >> index ad95659795..c722f9a897 100644
> >> --- a/libavformat/protocols.c
> >> +++ b/libavformat/protocols.c
> >> @@ -91,9 +91,13 @@ const AVClass
> *ff_urlcontext_child_class_next(const AVClass *prev)
> >>  }
> >>
> >>
> >> +#if FF_API_NONCONST_ENUM_PROTOCOLS
> >>  const char *avio_enum_protocols(void **opaque, int output)
> >> +#else
> >> +const char *avio_enum_protocols(const void **opaque, int output)
> >> +#endif
> >>  {
> >> -    const URLProtocol **p = *opaque;
> >> +    const URLProtocol * const *p = *opaque;
> >>
> >>      p = p ? p + 1 : url_protocols;
> >>      *opaque = p;
> >> diff --git a/libavformat/version.h b/libavformat/version.h
> >> index 9814db8633..b0b9264382 100644
> >> --- a/libavformat/version.h
> >> +++ b/libavformat/version.h
> >> @@ -106,6 +106,9 @@
> >>  #ifndef FF_API_AVIOFORMAT
> >>  #define FF_API_AVIOFORMAT               (LIBAVFORMAT_VERSION_MAJOR
> < 59)
> >>  #endif
> >> +#ifndef FF_API_NONCONST_ENUM_PROTOCOLS
> >> +#define FF_API_NONCONST_ENUM_PROTOCOLS
> (LIBAVFORMAT_VERSION_MAJOR < 59)
> >> +#endif
> >>
> >>
> >>  #ifndef FF_API_R_FRAME_RATE
> >> I am unsure what to do next given the feedback I received: If ABI
> > compability is no problem, then should I simply add the const and add
> > an entry to doc/APIchanges? Or is a deprecation period necessary?
> >
> > - Andreas
> >
> Ping for all three patches.
> 

Is it necessary to update the relevant code who calls avio_enum_protocols()
when this change takes effect? (either waiting for major version updates to 59,
or applied this modification right now)

Otherwise functions like show_protocols() may report warnings:

Compile with FF_API_NONCONST_ENUM_PROTOCOLS  forced to be 0 (gcc-7.3.0)

Message:
fftools/cmdutils.c: In function ‘show_protocols’:
fftools/cmdutils.c:1681:40: warning: passing argument 1 of ‘avio_enum_protocols’ from incompatible pointer type [-Wincompatible-pointer-types]
     while ((name = avio_enum_protocols(&opaque, 0)))
                                        ^
In file included from ./libavformat/avformat.h:321:0,
                 from fftools/cmdutils.c:34:
./libavformat/avio.h:811:13: note: expected ‘const void **’ but argument is of type ‘void **’
 const char *avio_enum_protocols(const void **opaque, int output);
             ^~~~~~~~~~~~~~~~~~~
fftools/cmdutils.c:1684:40: warning: passing argument 1 of ‘avio_enum_protocols’ from incompatible pointer type [-Wincompatible-pointer-types]
     while ((name = avio_enum_protocols(&opaque, 1)))
                                        ^
In file included from ./libavformat/avformat.h:321:0,
                 from fftools/cmdutils.c:34:
./libavformat/avio.h:811:13: note: expected ‘const void **’ but argument is of type ‘void **’
 const char *avio_enum_protocols(const void **opaque, int output);

- Linjie