[FFmpeg-devel] [PATCH 0/3] libavformat/protocols.c: avio_enum_protocols(): Cleanup

[Michael Witten]

This series improves the following function:

  libavformat/protocols.c: avio_enum_protocols()

There are really only 3 logical commits:

  [1] Add const-correctness     (fixes a compile-time warning)
  {2} Refactoring               (cleanup)
  [3] Add functions to the API  (use them too)

However, {2} is presented as a bunch of tiny little transformations
that are intended to aid comprehension; they can be squashed into
one commit as the maintainer sees fit (indeed, as shown below, the
squashed diff is already quite comprehensible):

  {2} Refactoring
        [2.0] Add more const-correctness
        [2.1] Split declaration and initialization
        [2.2] Convert recursion to iteration
        [2.3] Move 'iterate' label
        [2.4] Consolidate initialization of 'p'
        [2.5] Add block curly braces to 'if' statement
        [2.6] Move assignment to '*opaque'
        [2.7] Indent code
        [2.8] Make the 'else' logic explicit
        [2.9] Reverse the conditional
        [2.a] Move branch to bottom of function
        [2.b] Convert the 'goto' loop to a 'for(;;)' block
        [2.c] Move the loop variables
        [2.d] Move loop initialization
        [2.e] Create a macro for generating different loops

For your convenience, this email itself provides an example squashing
of those commits; to apply it, do something like the following:

  $ git am            /path/to/patch-1.eml   # Patch [1]
  $ git am --scissors /path/to/this-email    # This email's Patch {2}
  $ git am            /path/to/patch-3.eml   # Patch [3]

Alternatively, you can have the best of both worlds by means of a merge:

  $ git am /path/to/patch-1.eml              # Patch [1]
  $ patch_1=$(git rev-parse HEAD)
  $ git am /path/to/patch-2.*.eml            # Patches [2.*]
  $ git reset --hard "$patch_1"
  $ git merge --no-ff -m \
      'libavformat/protocols.c: avio_enum_protocols(): Refactor' \
      ORIG_HEAD
  $ git am /path/to/patch-3.eml              # Patch [3]

That will produce a very nice history:

  $ git log --oneline --graph

Here is the overall change for the entire series:

 fftools/cmdutils.c      |  4 ++--
 libavformat/avio.h      | 24 +++++++++++++++++++++++-
 libavformat/protocols.c | 36 +++++++++++++++++++++++++-----------
 3 files changed, 50 insertions(+), 14 deletions(-)

Sincerely,
Michael Witten

--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--

Subject: libavformat/protocols.c: avio_enum_protocols(): Refactor
Date: Wed, 11 Aug 2021 19:00:09 -0000

This commit is the result of squashing a series of very tiny
transformations; it refactors 'avio_enum_protocols()' into
2 functions:

  * avio_enum_protocols_for_input()
  * avio_enum_protocols_for_output()

Those functions are in turn mostly implemented by this macro:

  * AVIO_ENUM_PROTOCOLS()

The goal of this refactoring was the following:

  * To make the code more immediately understandable.
  * To reduce the potential for redundant computation.

diff --git a/libavformat/avio.h b/libavformat/avio.h
index 0b35409787..3b92cf742a 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -786,7 +786,7 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer);
  *
  * @return A static string containing the name of current protocol or NULL
  */
-const char *avio_enum_protocols(void **opaque, int output);
+const char *avio_enum_protocols(void **const opaque, const int output);
 
 /**
  * Get AVClass by names of available protocols.
diff --git a/libavformat/protocols.c b/libavformat/protocols.c
index e0b3405ab8..4cb8ae0b63 100644
--- a/libavformat/protocols.c
+++ b/libavformat/protocols.c
@@ -91,19 +91,35 @@ const AVClass *ff_urlcontext_child_class_iterate(void **iter)
     return ret;
 }
 
-const char *avio_enum_protocols(void **opaque, int output)
+#define AVIO_ENUM_PROTOCOLS(METHOD) \
+    typedef const URLProtocol *const *Iterator; \
+    for(Iterator p = *opaque ? (Iterator)(*opaque) + 1 : url_protocols; *p; ++p) { \
+        if ((*p)->METHOD) { \
+            *opaque = (void *)p; \
+            return (*p)->name; \
+        } \
+    } \
+    *opaque = NULL; \
+    return NULL;
+
+static inline
+const char *avio_enum_protocols_for_output(void **const opaque)
 {
-    const URLProtocol *const *p = *opaque;
+    AVIO_ENUM_PROTOCOLS(url_write);
+}
 
-    p = p ? p + 1 : url_protocols;
-    *opaque = (void *)p;
-    if (!*p) {
-        *opaque = NULL;
-        return NULL;
-    }
-    if ((output && (*p)->url_write) || (!output && (*p)->url_read))
-        return (*p)->name;
-    return avio_enum_protocols(opaque, output);
+static inline
+const char *avio_enum_protocols_for_input(void **const opaque)
+{
+    AVIO_ENUM_PROTOCOLS(url_read);
+}
+
+const char *avio_enum_protocols(void **const opaque, const int output)
+{
+    if (output)
+        return avio_enum_protocols_for_output(opaque);
+    else
+        return avio_enum_protocols_for_input(opaque);
 }
 
 const AVClass *avio_protocol_get_class(const char *name)