diff mbox series

[FFmpeg-devel,v3,4/5] avformat: introduce AVFormatContext io_close2 which returns an int

Message ID 20211204211544.26406-1-cus@passwd.hu
State New
Headers show
Series None | expand

Commit Message

Marton Balint Dec. 4, 2021, 9:15 p.m. UTC
Otherwise there is no way to detect an error returned by avio_close() because
ff_format_io_close cannot get the return value.

Checking the return value of the close function is important in order to check
if all data was successfully written and the underlying close() operation was
successful.

It can also be useful even for read mode because it can return any pending
AVIOContext error, so the user don't have to manually check AVIOContext->error.

In order to still support if the user overrides io_close, the generic code only
uses io_close2 if io_close is either NULL or the default io_close callback.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/APIchanges         |  3 +++
 libavformat/avformat.h | 13 +++++++++++++
 libavformat/dashenc.c  |  1 +
 libavformat/fifo.c     |  1 +
 libavformat/hlsenc.c   |  1 +
 libavformat/internal.h | 10 +++++++++-
 libavformat/options.c  |  7 ++++---
 libavformat/segment.c  |  1 +
 libavformat/tee.c      |  1 +
 libavformat/utils.c    | 17 ++++++++++++++---
 libavformat/version.h  |  4 ++--
 11 files changed, 50 insertions(+), 9 deletions(-)

Comments

Marton Balint Dec. 10, 2021, 11:22 p.m. UTC | #1
On Sat, 4 Dec 2021, Marton Balint wrote:

> Otherwise there is no way to detect an error returned by avio_close() because
> ff_format_io_close cannot get the return value.
>
> Checking the return value of the close function is important in order to check
> if all data was successfully written and the underlying close() operation was
> successful.
>
> It can also be useful even for read mode because it can return any pending
> AVIOContext error, so the user don't have to manually check AVIOContext->error.
>
> In order to still support if the user overrides io_close, the generic code only
> uses io_close2 if io_close is either NULL or the default io_close callback.
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> doc/APIchanges         |  3 +++
> libavformat/avformat.h | 13 +++++++++++++
> libavformat/dashenc.c  |  1 +
> libavformat/fifo.c     |  1 +
> libavformat/hlsenc.c   |  1 +
> libavformat/internal.h | 10 +++++++++-
> libavformat/options.c  |  7 ++++---
> libavformat/segment.c  |  1 +
> libavformat/tee.c      |  1 +
> libavformat/utils.c    | 17 ++++++++++++++---
> libavformat/version.h  |  4 ++--
> 11 files changed, 50 insertions(+), 9 deletions(-)

Will apply the series soon.

Regards,
Marton

>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 2914ad6734..a9092d27ef 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>
> API changes, most recent first:
>
> +2021-12-xx - xxxxxxxxxx - lavf 59.10.100 - avformat.h
> +  Add AVFormatContext io_close2 which returns an int
> +
> 2021-11-xx - xxxxxxxxxx - lavfi 8.19.100 - avfilter.h
>   Add AVFILTER_FLAG_METADATA_ONLY.
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 75699f3a32..70b36d7682 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1786,6 +1786,19 @@ typedef struct AVFormatContext {
>      * - decoding: set by user
>      */
>     int max_probe_packets;
> +
> +    /**
> +     * A callback for closing the streams opened with AVFormatContext.io_open().
> +     *
> +     * Using this is preferred over io_close, because this can return an error.
> +     * Therefore this callback is used instead of io_close by the generic
> +     * libavformat code if io_close is NULL or the default.
> +     *
> +     * @param s the format context
> +     * @param pb IO context to be closed and freed
> +     * @return 0 on success, a negative AVERROR code on failure
> +     */
> +    int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> } AVFormatContext;
>
> /**
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 3f28f5ad71..4709bc6615 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -1546,6 +1546,7 @@ static int dash_init(AVFormatContext *s)
>         ctx->interrupt_callback    = s->interrupt_callback;
>         ctx->opaque                = s->opaque;
>         ctx->io_close              = s->io_close;
> +        ctx->io_close2             = s->io_close2;
>         ctx->io_open               = s->io_open;
>         ctx->strict_std_compliance = s->strict_std_compliance;
>
> diff --git a/libavformat/fifo.c b/libavformat/fifo.c
> index 2ee6dde830..86e5d369b5 100644
> --- a/libavformat/fifo.c
> +++ b/libavformat/fifo.c
> @@ -499,6 +499,7 @@ static int fifo_mux_init(AVFormatContext *avf, const AVOutputFormat *oformat,
>         return ret;
>     avf2->opaque = avf->opaque;
>     avf2->io_close = avf->io_close;
> +    avf2->io_close2 = avf->io_close2;
>     avf2->io_open = avf->io_open;
>     avf2->flags = avf->flags;
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index cfd0c036d1..ae288e408d 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -867,6 +867,7 @@ static int hls_mux_init(AVFormatContext *s, VariantStream *vs)
>     oc->opaque                   = s->opaque;
>     oc->io_open                  = s->io_open;
>     oc->io_close                 = s->io_close;
> +    oc->io_close2                = s->io_close2;
>     oc->strict_std_compliance    = s->strict_std_compliance;
>     av_dict_copy(&oc->metadata, s->metadata, 0);
>
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 20e93d9267..b0f70c614b 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -905,8 +905,16 @@ int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op
> /*
>  * A wrapper around AVFormatContext.io_close that should be used
>  * instead of calling the pointer directly.
> + *
> + * @param s AVFormatContext
> + * @param *pb the AVIOContext to be closed and freed. Can be NULL.
> + * @return >=0 on success, negative AVERROR in case of failure
>  */
> -void ff_format_io_close(AVFormatContext *s, AVIOContext **pb);
> +int ff_format_io_close(AVFormatContext *s, AVIOContext **pb);
> +
> +/* Default io_close callback, not to be used directly, use ff_format_io_close
> + * instead. */
> +void ff_format_io_close_default(AVFormatContext *s, AVIOContext *pb);
>
> /**
>  * Utility function to check if the file uses http or https protocol
> diff --git a/libavformat/options.c b/libavformat/options.c
> index 72c9bdcefe..1634388acb 100644
> --- a/libavformat/options.c
> +++ b/libavformat/options.c
> @@ -146,9 +146,9 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb,
>     return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist);
> }
>
> -static void io_close_default(AVFormatContext *s, AVIOContext *pb)
> +static int io_close2_default(AVFormatContext *s, AVIOContext *pb)
> {
> -    avio_close(pb);
> +    return avio_close(pb);
> }
>
> AVFormatContext *avformat_alloc_context(void)
> @@ -162,7 +162,8 @@ AVFormatContext *avformat_alloc_context(void)
>     s = &si->pub;
>     s->av_class = &av_format_context_class;
>     s->io_open  = io_open_default;
> -    s->io_close = io_close_default;
> +    s->io_close = ff_format_io_close_default;
> +    s->io_close2= io_close2_default;
>
>     av_opt_set_defaults(s);
>
> diff --git a/libavformat/segment.c b/libavformat/segment.c
> index d531286c31..e9b0aa4fa8 100644
> --- a/libavformat/segment.c
> +++ b/libavformat/segment.c
> @@ -158,6 +158,7 @@ static int segment_mux_init(AVFormatContext *s)
>     av_dict_copy(&oc->metadata, s->metadata, 0);
>     oc->opaque             = s->opaque;
>     oc->io_close           = s->io_close;
> +    oc->io_close2          = s->io_close2;
>     oc->io_open            = s->io_open;
>     oc->flags              = s->flags;
>
> diff --git a/libavformat/tee.c b/libavformat/tee.c
> index dfdf70cb08..b3bcd70b9f 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -237,6 +237,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>     avf2->opaque   = avf->opaque;
>     avf2->io_open  = avf->io_open;
>     avf2->io_close = avf->io_close;
> +    avf2->io_close2 = avf->io_close2;
>     avf2->interrupt_callback = avf->interrupt_callback;
>     avf2->flags = avf->flags;
>     avf2->strict_std_compliance = avf->strict_std_compliance;
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 7840e8717c..e449c45fec 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1829,11 +1829,22 @@ int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op
>     return 0;
> }
>
> -void ff_format_io_close(AVFormatContext *s, AVIOContext **pb)
> +void ff_format_io_close_default(AVFormatContext *s, AVIOContext *pb)
> {
> -    if (*pb)
> -        s->io_close(s, *pb);
> +    avio_close(pb);
> +}
> +
> +int ff_format_io_close(AVFormatContext *s, AVIOContext **pb)
> +{
> +    int ret = 0;
> +    if (*pb) {
> +        if (s->io_close == ff_format_io_close_default || s->io_close == NULL)
> +            ret = s->io_close2(s, *pb);
> +        else
> +            s->io_close(s, *pb);
> +    }
>     *pb = NULL;
> +    return ret;
> }
>
> int ff_is_http_proto(const char *filename) {
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 0705ee4112..1623835a78 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -32,8 +32,8 @@
> // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
> // Also please add any ticket numbers that you believe might be affected here
> #define LIBAVFORMAT_VERSION_MAJOR  59
> -#define LIBAVFORMAT_VERSION_MINOR   9
> -#define LIBAVFORMAT_VERSION_MICRO 102
> +#define LIBAVFORMAT_VERSION_MINOR  10
> +#define LIBAVFORMAT_VERSION_MICRO 100
>
> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>                                                LIBAVFORMAT_VERSION_MINOR, \
> -- 
> 2.31.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 2914ad6734..a9092d27ef 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@  libavutil:     2021-04-27
 
 API changes, most recent first:
 
+2021-12-xx - xxxxxxxxxx - lavf 59.10.100 - avformat.h
+  Add AVFormatContext io_close2 which returns an int
+
 2021-11-xx - xxxxxxxxxx - lavfi 8.19.100 - avfilter.h
   Add AVFILTER_FLAG_METADATA_ONLY.
 
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 75699f3a32..70b36d7682 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1786,6 +1786,19 @@  typedef struct AVFormatContext {
      * - decoding: set by user
      */
     int max_probe_packets;
+
+    /**
+     * A callback for closing the streams opened with AVFormatContext.io_open().
+     *
+     * Using this is preferred over io_close, because this can return an error.
+     * Therefore this callback is used instead of io_close by the generic
+     * libavformat code if io_close is NULL or the default.
+     *
+     * @param s the format context
+     * @param pb IO context to be closed and freed
+     * @return 0 on success, a negative AVERROR code on failure
+     */
+    int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
 } AVFormatContext;
 
 /**
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 3f28f5ad71..4709bc6615 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -1546,6 +1546,7 @@  static int dash_init(AVFormatContext *s)
         ctx->interrupt_callback    = s->interrupt_callback;
         ctx->opaque                = s->opaque;
         ctx->io_close              = s->io_close;
+        ctx->io_close2             = s->io_close2;
         ctx->io_open               = s->io_open;
         ctx->strict_std_compliance = s->strict_std_compliance;
 
diff --git a/libavformat/fifo.c b/libavformat/fifo.c
index 2ee6dde830..86e5d369b5 100644
--- a/libavformat/fifo.c
+++ b/libavformat/fifo.c
@@ -499,6 +499,7 @@  static int fifo_mux_init(AVFormatContext *avf, const AVOutputFormat *oformat,
         return ret;
     avf2->opaque = avf->opaque;
     avf2->io_close = avf->io_close;
+    avf2->io_close2 = avf->io_close2;
     avf2->io_open = avf->io_open;
     avf2->flags = avf->flags;
 
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index cfd0c036d1..ae288e408d 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -867,6 +867,7 @@  static int hls_mux_init(AVFormatContext *s, VariantStream *vs)
     oc->opaque                   = s->opaque;
     oc->io_open                  = s->io_open;
     oc->io_close                 = s->io_close;
+    oc->io_close2                = s->io_close2;
     oc->strict_std_compliance    = s->strict_std_compliance;
     av_dict_copy(&oc->metadata, s->metadata, 0);
 
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 20e93d9267..b0f70c614b 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -905,8 +905,16 @@  int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op
 /*
  * A wrapper around AVFormatContext.io_close that should be used
  * instead of calling the pointer directly.
+ *
+ * @param s AVFormatContext
+ * @param *pb the AVIOContext to be closed and freed. Can be NULL.
+ * @return >=0 on success, negative AVERROR in case of failure
  */
-void ff_format_io_close(AVFormatContext *s, AVIOContext **pb);
+int ff_format_io_close(AVFormatContext *s, AVIOContext **pb);
+
+/* Default io_close callback, not to be used directly, use ff_format_io_close
+ * instead. */
+void ff_format_io_close_default(AVFormatContext *s, AVIOContext *pb);
 
 /**
  * Utility function to check if the file uses http or https protocol
diff --git a/libavformat/options.c b/libavformat/options.c
index 72c9bdcefe..1634388acb 100644
--- a/libavformat/options.c
+++ b/libavformat/options.c
@@ -146,9 +146,9 @@  static int io_open_default(AVFormatContext *s, AVIOContext **pb,
     return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist);
 }
 
-static void io_close_default(AVFormatContext *s, AVIOContext *pb)
+static int io_close2_default(AVFormatContext *s, AVIOContext *pb)
 {
-    avio_close(pb);
+    return avio_close(pb);
 }
 
 AVFormatContext *avformat_alloc_context(void)
@@ -162,7 +162,8 @@  AVFormatContext *avformat_alloc_context(void)
     s = &si->pub;
     s->av_class = &av_format_context_class;
     s->io_open  = io_open_default;
-    s->io_close = io_close_default;
+    s->io_close = ff_format_io_close_default;
+    s->io_close2= io_close2_default;
 
     av_opt_set_defaults(s);
 
diff --git a/libavformat/segment.c b/libavformat/segment.c
index d531286c31..e9b0aa4fa8 100644
--- a/libavformat/segment.c
+++ b/libavformat/segment.c
@@ -158,6 +158,7 @@  static int segment_mux_init(AVFormatContext *s)
     av_dict_copy(&oc->metadata, s->metadata, 0);
     oc->opaque             = s->opaque;
     oc->io_close           = s->io_close;
+    oc->io_close2          = s->io_close2;
     oc->io_open            = s->io_open;
     oc->flags              = s->flags;
 
diff --git a/libavformat/tee.c b/libavformat/tee.c
index dfdf70cb08..b3bcd70b9f 100644
--- a/libavformat/tee.c
+++ b/libavformat/tee.c
@@ -237,6 +237,7 @@  static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
     avf2->opaque   = avf->opaque;
     avf2->io_open  = avf->io_open;
     avf2->io_close = avf->io_close;
+    avf2->io_close2 = avf->io_close2;
     avf2->interrupt_callback = avf->interrupt_callback;
     avf2->flags = avf->flags;
     avf2->strict_std_compliance = avf->strict_std_compliance;
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 7840e8717c..e449c45fec 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1829,11 +1829,22 @@  int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op
     return 0;
 }
 
-void ff_format_io_close(AVFormatContext *s, AVIOContext **pb)
+void ff_format_io_close_default(AVFormatContext *s, AVIOContext *pb)
 {
-    if (*pb)
-        s->io_close(s, *pb);
+    avio_close(pb);
+}
+
+int ff_format_io_close(AVFormatContext *s, AVIOContext **pb)
+{
+    int ret = 0;
+    if (*pb) {
+        if (s->io_close == ff_format_io_close_default || s->io_close == NULL)
+            ret = s->io_close2(s, *pb);
+        else
+            s->io_close(s, *pb);
+    }
     *pb = NULL;
+    return ret;
 }
 
 int ff_is_http_proto(const char *filename) {
diff --git a/libavformat/version.h b/libavformat/version.h
index 0705ee4112..1623835a78 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,8 +32,8 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  59
-#define LIBAVFORMAT_VERSION_MINOR   9
-#define LIBAVFORMAT_VERSION_MICRO 102
+#define LIBAVFORMAT_VERSION_MINOR  10
+#define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
                                                LIBAVFORMAT_VERSION_MINOR, \