[FFmpeg-devel] [PATCH 2/2] avio: do not export avpriv_io_{move, delete}

Hendrik Leppkes h.leppkes at gmail.com
Sat Jan 11 00:40:37 EET 2020


On Fri, Jan 10, 2020 at 7:31 PM James Almer <jamrial at gmail.com> wrote:
>
> On 1/10/2020 11:30 AM, Anton Khirnov wrote:
> > Quoting James Almer (2020-01-10 14:33:06)
> >> On 1/10/2020 8:21 AM, Anton Khirnov wrote:
> >>> They are private and not used by anything outside of lavf. There is no
> >>> reason for them to be exported.
> >>> ---
> >>>  libavformat/avio.c    |  4 ++--
> >>>  libavformat/avio.h    | 19 -------------------
> >>>  libavformat/dashenc.c | 10 +++++-----
> >>>  libavformat/url.h     | 20 ++++++++++++++++++++
> >>>  4 files changed, 27 insertions(+), 26 deletions(-)
> >>>
> >>> diff --git a/libavformat/avio.c b/libavformat/avio.c
> >>> index 2dd2312296..3e390fe719 100644
> >>> --- a/libavformat/avio.c
> >>> +++ b/libavformat/avio.c
> >>> @@ -494,7 +494,7 @@ int avio_check(const char *url, int flags)
> >>>      return ret;
> >>>  }
> >>>
> >>> -int avpriv_io_move(const char *url_src, const char *url_dst)
> >>> +int ffurl_move(const char *url_src, const char *url_dst)
> >>>  {
> >>>      URLContext *h_src, *h_dst;
> >>>      int ret = ffurl_alloc(&h_src, url_src, AVIO_FLAG_READ_WRITE, NULL);
> >>> @@ -516,7 +516,7 @@ int avpriv_io_move(const char *url_src, const char *url_dst)
> >>>      return ret;
> >>>  }
> >>>
> >>> -int avpriv_io_delete(const char *url)
> >>> +int ffurl_delete(const char *url)
> >>>  {
> >>>      URLContext *h;
> >>>      int ret = ffurl_alloc(&h, url, AVIO_FLAG_WRITE, NULL);
> >>> diff --git a/libavformat/avio.h b/libavformat/avio.h
> >>> index 9141642e75..34c5957791 100644
> >>> --- a/libavformat/avio.h
> >>> +++ b/libavformat/avio.h
> >>> @@ -374,25 +374,6 @@ const char *avio_find_protocol_name(const char *url);
> >>>   */
> >>>  int avio_check(const char *url, int flags);
> >>>
> >>> -/**
> >>> - * Move or rename a resource.
> >>> - *
> >>> - * @note url_src and url_dst should share the same protocol and authority.
> >>> - *
> >>> - * @param url_src url to resource to be moved
> >>> - * @param url_dst new url to resource if the operation succeeded
> >>> - * @return >=0 on success or negative on error.
> >>> - */
> >>> -int avpriv_io_move(const char *url_src, const char *url_dst);
> >>> -
> >>> -/**
> >>> - * Delete a resource.
> >>> - *
> >>> - * @param url resource to be deleted.
> >>> - * @return >=0 on success or negative on error.
> >>> - */
> >>> -int avpriv_io_delete(const char *url);
> >>
> >> No, unfortunately and despite the name, these are public. Or rather,
> >> exposed in a public header when they were not meant to be public,
> >> afaics, so what we can do instead is schedule them for removal in the
> >> next bump and not bother with a two year deprecation period. We've done
> >> it before for other avpriv_ functions mistakenly exposed in public headers.
> >
> > There's (sadly) plenty of stuff in public headers that is not meant to
> > be public. For example, the FF_API_* macros, the internal fields of
> > structs like AVCodec, etc. IMO we should not take into consideration
> > people using private APIs, otherwise we legitimize such behavior. Anyone
> > using a private function gets to keep all the pieces when we break it.
>
> Yes, i agree with this if we were talking about clearly marked
> non-public structs, fields and defines. In those cases, people using
> them should be aware of the consequences. But these are two five years
> old documented functions in a public header (Although admittedly not
> announced in APIChanges) and even featured in an API example tool.
> I don't think the avpriv_ prefix is something we documented outside of
> the developer guidelines, so anyone reading avio.h and the example will
> have no reason to think these were there by mistake.
>
> You could replace the doxy with a "do not use" notice aside from
> wrapping them in a scheduled removal preprocessor check, but at the very
> least the symbols should not be removed until the next major bump.
>
> If others agree with you though, I'll not block this.
>

If we plan to do a bump relatively soon anyway, then there is no
reason to really rush anything, and just let them in until such a time
that there is a bump. Considering their only not-really-public nature
we can forego the 2 year deprecation and just throw them out when we
open the ABI.

- Hendrik


More information about the ffmpeg-devel mailing list