[FFmpeg-devel] [PATCH 3/3] libavformat/protocols.c: avio_enum_protocols_{input, output}(): Add functions to the API

[Andreas Rheinhardt]

Michael Witten:
> In the repo, there is only one function that enumerates protocols:
> 
>   fftools/cmdutils.c: show_protocols()
> 
> This commit simply has that function make calls directly to the
> desired functions, namely:
> 
>   * avio_enum_protocols_for_input()
>   * avio_enum_protocols_for_output()
> ---
>  fftools/cmdutils.c      |  4 ++--
>  libavformat/avio.h      | 22 ++++++++++++++++++++++
>  libavformat/protocols.c |  2 --
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 64237a4796..87f9be64c3 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -1681,10 +1681,10 @@ int show_protocols(void *optctx, const char *opt, const char *arg)
>  
>      printf("Supported file protocols:\n"
>             "Input:\n");
> -    while ((name = avio_enum_protocols(&opaque, 0)))
> +    while ((name = avio_enum_protocols_for_input(&opaque)))
>          printf("  %s\n", name);
>      printf("Output:\n");
> -    while ((name = avio_enum_protocols(&opaque, 1)))
> +    while ((name = avio_enum_protocols_for_output(&opaque)))

You did not reset opaque before the second call; instead you are relying
on undocumented behaviour, namely that opaque is reset to NULL when no
further protocol exists. (Instead the implementation could also make
opaque point to the sentinel of the array of protocols.) In fact, the
current code already relied on this, but with two functions it is worse,
because in the second loop the opaque for the second function does not
come from a call of avio_enum_protocols_for_output() at all.

>          printf("  %s\n", name);
>      return 0;
>  }
> diff --git a/libavformat/avio.h b/libavformat/avio.h
> index 3b92cf742a..455b872260 100644
> --- a/libavformat/avio.h
> +++ b/libavformat/avio.h
> @@ -788,6 +788,28 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer);
>   */
>  const char *avio_enum_protocols(void **const opaque, const int output);
>  
> +/**
> + * Iterate through names of available output protocols.
> + *
> + * @param opaque A private pointer representing current protocol.
> + *        It must be a pointer to NULL on first iteration and will
> + *        be updated by successive calls to avio_enum_protocols_for_output.
> + *
> + * @return A static string containing the name of current protocol or NULL
> + */
> +const char *avio_enum_protocols_for_output(void **const opaque);
> +
> +/**
> + * Iterate through names of available input protocols.
> + *
> + * @param opaque A private pointer representing current protocol.
> + *        It must be a pointer to NULL on first iteration and will
> + *        be updated by successive calls to avio_enum_protocols_for_input.
> + *
> + * @return A static string containing the name of current protocol or NULL
> + */
> +const char *avio_enum_protocols_for_input(void **const opaque);

I don't think it is worth adding two more public symbols for this.

> +
>  /**
>   * Get AVClass by names of available protocols.
>   *
> diff --git a/libavformat/protocols.c b/libavformat/protocols.c
> index 4cb8ae0b63..ca04ed2eb5 100644
> --- a/libavformat/protocols.c
> +++ b/libavformat/protocols.c
> @@ -102,13 +102,11 @@ const AVClass *ff_urlcontext_child_class_iterate(void **iter)
>      *opaque = NULL; \
>      return NULL;
>  
> -static inline
>  const char *avio_enum_protocols_for_output(void **const opaque)
>  {
>      AVIO_ENUM_PROTOCOLS(url_write);
>  }
>  
> -static inline
>  const char *avio_enum_protocols_for_input(void **const opaque)
>  {
>      AVIO_ENUM_PROTOCOLS(url_read);
>