diff mbox series

[FFmpeg-devel,RFC/PATCH] avcodec: deprecate thread_safe_callbacks

Message ID 20200522140441.32421-1-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,RFC/PATCH] avcodec: deprecate thread_safe_callbacks | expand

Checks

Context Check Description
andriy/default pending
andriy/make_warn warning New warnings during build
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Anton Khirnov May 22, 2020, 2:04 p.m. UTC 
They add considerable complexity to frame-threading implementation,
which includes an unavoidably leaking error path, while the advantages
of this option to the users are highly dubious.

It should be always possible and desirable for the callers to make their
get_buffer2() implementation thread-safe, so deprecate this option.
---
The patch is incomplete and not ready to be pushed, I am mainly
gathering opinions. Is anyone aware of a valid use case for
thread_safe_callbacks?
---
 libavcodec/avcodec.h |  7 +++++++
 libavcodec/utils.c   | 13 +++++++++++++
 libavcodec/version.h |  3 +++
 3 files changed, 23 insertions(+)

Comments

Lynne May 22, 2020, 2:18 p.m. UTC | #1 
May 22, 2020, 15:04 by anton@khirnov.net:

> They add considerable complexity to frame-threading implementation,
> which includes an unavoidably leaking error path, while the advantages
> of this option to the users are highly dubious.
>
> It should be always possible and desirable for the callers to make their
> get_buffer2() implementation thread-safe, so deprecate this option.
> ---
> The patch is incomplete and not ready to be pushed, I am mainly
> gathering opinions. Is anyone aware of a valid use case for
> thread_safe_callbacks?
>

+1
mpv's get_buffer2 implementation is thread safe yet still doesn't signal its thread-safe due
to misunderstandings with the docs for no good reason.
API users have always had the option to make their implementation thread safe so I'm not
sure why this was even an option.
diff mbox series

Patch

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index f10b7a06ec..2dec0d8ca0 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1934,6 +1934,7 @@  typedef struct AVCodecContext {
      */
     int active_thread_type;
 
+#if FF_API_THREAD_SAFE_CALLBACKS
     /**
      * Set by the client if its custom get_buffer() callback can be called
      * synchronously from another thread, which allows faster multithreaded decoding.
@@ -1941,8 +1942,14 @@  typedef struct AVCodecContext {
      * Ignored if the default get_buffer() is used.
      * - encoding: Set by user.
      * - decoding: Set by user.
+     *
+     * @deprecated the custom get_buffer2() callback should always be
+     *   thread-safe. Thread-unsafe get_buffer2() implementations will be
+     *   invalid once this field is removed.
      */
+    attribute_deprecated
     int thread_safe_callbacks;
+#endif
 
     /**
      * The codec may call this to execute several independent things.
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 3255679550..10d5e552c2 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -673,6 +673,19 @@  int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
         goto free_and_end;
     }
 
+#if FF_API_THREAD_SAFE_CALLBACKS
+FF_DISABLE_DEPRECATION_WARNINGS
+    if ((avctx->thread_type & FF_THREAD_FRAME) &&
+        avctx->get_buffer2 != avcodec_default_get_buffer2 &&
+        !avctx->thread_safe_callbacks) {
+        av_log(avctx, AV_LOG_WARNING, "Requested frame threading with a "
+               "custom get_buffer2() implementation which is not marked as "
+               "thread safe. This is not supported anymore, make your "
+               "callback thread-safe.\n");
+    }
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
     avctx->codec = codec;
     if ((avctx->codec_type == AVMEDIA_TYPE_UNKNOWN || avctx->codec_type == codec->type) &&
         avctx->codec_id == AV_CODEC_ID_NONE) {
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 9fc637313d..5b4dfea579 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -144,6 +144,9 @@ 
 #ifndef FF_API_UNUSED_CODEC_CAPS
 #define FF_API_UNUSED_CODEC_CAPS   (LIBAVCODEC_VERSION_MAJOR < 59)
 #endif
+#ifndef FF_API_THREAD_SAFE_CALLBACKS
+#define FF_API_THREAD_SAFE_CALLBACKS (LIBAVCODEC_VERSION_MAJOR < 59)
+#endif
 
 
 #endif /* AVCODEC_VERSION_H */