diff mbox series

[FFmpeg-devel,06/10] avfilter: Avoid allocation of AVFilterGraphInternal

Message ID AM7PR03MB6660212F7110D4AD6970E37B8FF89@AM7PR03MB6660.eurprd03.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel,1/3] avfilter/avfilter: Fix leaks upon filter creation error | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Aug. 11, 2021, 1:17 a.m. UTC 
This can be achieved by allocating it together with AVFilterGraph,
with the public AVFilterGraph as first element in the new structure.
Given that said structure is now more than just the internal, it has
been renamed to FFFilterGraph.
Accessing it is not type-safe, so this unsafety has been confined to
a single function.

This breaks ABI due to the removal of the removal of the
AVFilterGraph.internal pointer.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
This one is fairly easy due to the internal not being used much;
I also followed Nicolas suggestion to factor the unsafe code out into
a dedicated function.

 libavfilter/avfilter.c      |  6 +++---
 libavfilter/avfilter.h      |  7 -------
 libavfilter/avfiltergraph.c | 21 +++++++--------------
 libavfilter/internal.h      | 10 ++++++++--
 libavfilter/pthread.c       | 20 +++++++++++---------
 5 files changed, 29 insertions(+), 35 deletions(-)
diff mbox series

Patch

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index c04aefcaa8..7e48ff14c5 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -167,7 +167,7 @@  int avfilter_link(AVFilterContext *src, unsigned srcpad,
     link->type    = src->output_pads[srcpad].type;
     av_assert0(AV_PIX_FMT_NONE == -1 && AV_SAMPLE_FMT_NONE == -1);
     link->format  = -1;
-    ff_framequeue_init(&link->fifo, &src->graph->internal->frame_queues);
+    ff_framequeue_init(&link->fifo, &filtergraph(src->graph)->frame_queues);
 
     return 0;
 }
@@ -868,9 +868,9 @@  int avfilter_init_dict(AVFilterContext *ctx, AVDictionary **options)
 
     if (ctx->filter->flags & AVFILTER_FLAG_SLICE_THREADS &&
         ctx->thread_type & ctx->graph->thread_type & AVFILTER_THREAD_SLICE &&
-        ctx->graph->internal->thread_execute) {
+        filtergraph(ctx->graph)->thread_execute) {
         ctx->thread_type       = AVFILTER_THREAD_SLICE;
-        ctx->internal->execute = ctx->graph->internal->thread_execute;
+        ctx->internal->execute = filtergraph(ctx->graph)->thread_execute;
     } else {
         ctx->thread_type = 0;
     }
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index 360f63bc45..4436eb7d2d 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -758,8 +758,6 @@  int avfilter_insert_filter(AVFilterLink *link, AVFilterContext *filt,
  */
 const AVClass *avfilter_get_class(void);
 
-typedef struct AVFilterGraphInternal AVFilterGraphInternal;
-
 /**
  * A function pointer passed to the @ref AVFilterGraph.execute callback to be
  * executed multiple times, possibly in parallel.
@@ -817,11 +815,6 @@  typedef struct AVFilterGraph {
      */
     int nb_threads;
 
-    /**
-     * Opaque object for libavfilter internal use.
-     */
-    AVFilterGraphInternal *internal;
-
     /**
      * Opaque user data. May be set by the caller to an arbitrary value, e.g. to
      * be used from callbacks like @ref AVFilterGraph.execute.
diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index f6bbcd8578..f8a2426c46 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -82,21 +82,15 @@  int ff_graph_thread_init(AVFilterGraph *graph)
 
 AVFilterGraph *avfilter_graph_alloc(void)
 {
-    AVFilterGraph *ret = av_mallocz(sizeof(*ret));
+    FFFilterGraph *const ret = av_mallocz(sizeof(*ret));
     if (!ret)
         return NULL;
 
-    ret->internal = av_mallocz(sizeof(*ret->internal));
-    if (!ret->internal) {
-        av_freep(&ret);
-        return NULL;
-    }
+    ret->pub.av_class = &filtergraph_class;
+    av_opt_set_defaults(&ret->pub);
+    ff_framequeue_global_init(&ret->frame_queues);
 
-    ret->av_class = &filtergraph_class;
-    av_opt_set_defaults(ret);
-    ff_framequeue_global_init(&ret->internal->frame_queues);
-
-    return ret;
+    return &ret->pub;
 }
 
 void ff_filter_graph_remove_filter(AVFilterGraph *graph, AVFilterContext *filter)
@@ -132,7 +126,6 @@  void avfilter_graph_free(AVFilterGraph **graph)
     av_freep(&(*graph)->scale_sws_opts);
     av_freep(&(*graph)->aresample_swr_opts);
     av_freep(&(*graph)->filters);
-    av_freep(&(*graph)->internal);
     av_freep(graph);
 }
 
@@ -170,9 +163,9 @@  AVFilterContext *avfilter_graph_alloc_filter(AVFilterGraph *graph,
 {
     AVFilterContext **filters, *s;
 
-    if (graph->thread_type && !graph->internal->thread_execute) {
+    if (graph->thread_type && !filtergraph(graph)->thread_execute) {
         if (graph->execute) {
-            graph->internal->thread_execute = graph->execute;
+            filtergraph(graph)->thread_execute = graph->execute;
         } else {
             int ret = ff_graph_thread_init(graph);
             if (ret < 0) {
diff --git a/libavfilter/internal.h b/libavfilter/internal.h
index 6c908690b4..2d1dab5788 100644
--- a/libavfilter/internal.h
+++ b/libavfilter/internal.h
@@ -122,11 +122,17 @@  struct AVFilterPad {
     int needs_writable;
 };
 
-struct AVFilterGraphInternal {
+typedef struct FFFilterGraph {
+    AVFilterGraph pub;
     void *thread;
     avfilter_execute_func *thread_execute;
     FFFrameQueueGlobal frame_queues;
-};
+} FFFilterGraph;
+
+static av_always_inline FFFilterGraph *filtergraph(AVFilterGraph *graph)
+{
+    return (FFFilterGraph*)graph;
+}
 
 struct AVFilterInternal {
     avfilter_execute_func *execute;
diff --git a/libavfilter/pthread.c b/libavfilter/pthread.c
index 34fc699505..0b8e98f60e 100644
--- a/libavfilter/pthread.c
+++ b/libavfilter/pthread.c
@@ -59,7 +59,7 @@  static void slice_thread_uninit(ThreadContext *c)
 static int thread_execute(AVFilterContext *ctx, avfilter_action_func *func,
                           void *arg, int *ret, int nb_jobs)
 {
-    ThreadContext *c = ctx->graph->internal->thread;
+    ThreadContext *const c = filtergraph(ctx->graph)->thread;
 
     if (nb_jobs <= 0)
         return 0;
@@ -82,6 +82,7 @@  static int thread_init_internal(ThreadContext *c, int nb_threads)
 
 int ff_graph_thread_init(AVFilterGraph *graph)
 {
+    FFFilterGraph *const internal = filtergraph(graph);
     int ret;
 
     if (graph->nb_threads == 1) {
@@ -89,27 +90,28 @@  int ff_graph_thread_init(AVFilterGraph *graph)
         return 0;
     }
 
-    graph->internal->thread = av_mallocz(sizeof(ThreadContext));
-    if (!graph->internal->thread)
+    internal->thread = av_mallocz(sizeof(ThreadContext));
+    if (!internal->thread)
         return AVERROR(ENOMEM);
 
-    ret = thread_init_internal(graph->internal->thread, graph->nb_threads);
+    ret = thread_init_internal(internal->thread, graph->nb_threads);
     if (ret <= 1) {
-        av_freep(&graph->internal->thread);
+        av_freep(&internal->thread);
         graph->thread_type = 0;
         graph->nb_threads  = 1;
         return (ret < 0) ? ret : 0;
     }
     graph->nb_threads = ret;
 
-    graph->internal->thread_execute = thread_execute;
+    internal->thread_execute = thread_execute;
 
     return 0;
 }
 
 void ff_graph_thread_free(AVFilterGraph *graph)
 {
-    if (graph->internal->thread)
-        slice_thread_uninit(graph->internal->thread);
-    av_freep(&graph->internal->thread);
+    FFFilterGraph *const internal = filtergraph(graph);
+    if (internal->thread)
+        slice_thread_uninit(internal->thread);
+    av_freep(&internal->thread);
 }