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

Michael Witten mfwitten at gmail.com
Thu Aug 12 00:57:00 EEST 2021


| 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()
|   >
|   > [...]
|   > -    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)))
|
| Andreas Rheinhardt:
|
|   > 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.
|   >
|   > [...]
|   >
|   > I don't think it is worth adding two more public symbols for this.

* Indeed, I did not attempt to alter 'show_protocols()', other than to
  replace one API call with another, more direct API call.

* Perhaps it should simply be documented that 'opaque' is set to NULL.

* I would not be opposed to dropping this patch; it is just a potentiality.

Sincerely,
Michael WItten


More information about the ffmpeg-devel mailing list