diff mbox series

[FFmpeg-devel,01/10] fftools/ffmpeg_filter: stop using avfilter_graph_alloc_filter() incorrectly

Message ID 20240925132921.11203-1-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/10] fftools/ffmpeg_filter: stop using avfilter_graph_alloc_filter() incorrectly | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Sept. 25, 2024, 1:29 p.m. UTC 
This function creates AND initializes a filter, so setting any filter
options after it is wrong. It happens to work when the filter's init
function does not touch the options in question, but is forbidden by the
API and is not guaranteed to remain functional.

Instead, use avfilter_graph_alloc_filter(), followed by setting the
options, and avfilter_init_dict().
---
 fftools/ffmpeg_filter.c | 53 ++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

Comments

Anton Khirnov Sept. 25, 2024, 1:49 p.m. UTC | #1 
> fftools/ffmpeg_filter: stop using avfilter_graph_alloc_filter() incorrectly
                                                   ^^^^^
Should be s/alloc/create/, fixed locally in all the commits.

Thanks to ePirat for noticing.
James Almer Sept. 27, 2024, 10:27 p.m. UTC | #2 
On 9/25/2024 10:29 AM, Anton Khirnov wrote:
> This function creates AND initializes a filter, so setting any filter
> options after it is wrong. It happens to work when the filter's init
> function does not touch the options in question, but is forbidden by the
> API and is not guaranteed to remain functional.
> 
> Instead, use avfilter_graph_alloc_filter(), followed by setting the
> options, and avfilter_init_dict().

Shouldn't the subject be "stop using avfilter_graph_create_filter() 
incorrectly"?
Marvin Scholz Sept. 28, 2024, 1:09 a.m. UTC | #3 
On 28 Sep 2024, at 0:27, James Almer wrote:

> On 9/25/2024 10:29 AM, Anton Khirnov wrote:
>> This function creates AND initializes a filter, so setting any filter
>> options after it is wrong. It happens to work when the filter's init
>> function does not touch the options in question, but is forbidden by the
>> API and is not guaranteed to remain functional.
>>
>> Instead, use avfilter_graph_alloc_filter(), followed by setting the
>> options, and avfilter_init_dict().
>
> Shouldn't the subject be "stop using avfilter_graph_create_filter() incorrectly"?
>

See https://ffmpeg.org//pipermail/ffmpeg-devel/2024-September/334032.html

> _______________________________________________
> 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/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index c36a3ad00a..2f7597b491 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -1586,14 +1586,18 @@  static int configure_output_audio_filter(FilterGraph *fg, AVFilterGraph *graph,
     int ret;
 
     snprintf(name, sizeof(name), "out_%s", ofp->name);
-    ret = avfilter_graph_create_filter(&ofp->filter,
-                                       avfilter_get_by_name("abuffersink"),
-                                       name, NULL, NULL, graph);
-    if (ret < 0)
-        return ret;
+    ofp->filter = avfilter_graph_alloc_filter(graph,
+                                              avfilter_get_by_name("abuffersink"),
+                                              name);
+    if (!ofp->filter)
+        return AVERROR(ENOMEM);
     if ((ret = av_opt_set_int(ofp->filter, "all_channel_counts", 1, AV_OPT_SEARCH_CHILDREN)) < 0)
         return ret;
 
+    ret = avfilter_init_dict(ofp->filter, NULL);
+    if (ret < 0)
+        return ret;
+
 #define AUTO_INSERT_FILTER(opt_name, filter_name, arg) do {                 \
     AVFilterContext *filt_ctx;                                              \
                                                                             \
@@ -1686,9 +1690,6 @@  static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
     AVFilterContext *last_filter;
     const AVFilter *buffer_filt = avfilter_get_by_name("buffer");
     const AVPixFmtDescriptor *desc;
-    AVRational fr = ifp->opts.framerate;
-    AVRational sar;
-    AVBPrint args;
     char name[255];
     int ret, pad_idx = 0;
     AVBufferSrcParameters *par = av_buffersrc_parameters_alloc();
@@ -1698,30 +1699,34 @@  static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
     if (ifp->type_src == AVMEDIA_TYPE_SUBTITLE)
         sub2video_prepare(ifp);
 
-    sar = ifp->sample_aspect_ratio;
-    if(!sar.den)
-        sar = (AVRational){0,1};
-    av_bprint_init(&args, 0, AV_BPRINT_SIZE_AUTOMATIC);
-    av_bprintf(&args,
-             "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:"
-             "pixel_aspect=%d/%d:colorspace=%d:range=%d",
-             ifp->width, ifp->height, ifp->format,
-             ifp->time_base.num, ifp->time_base.den, sar.num, sar.den,
-             ifp->color_space, ifp->color_range);
-    if (fr.num && fr.den)
-        av_bprintf(&args, ":frame_rate=%d/%d", fr.num, fr.den);
     snprintf(name, sizeof(name), "graph %d input from stream %s", fg->index,
              ifp->opts.name);
 
-
-    if ((ret = avfilter_graph_create_filter(&ifp->filter, buffer_filt, name,
-                                            args.str, NULL, graph)) < 0)
+    ifp->filter = avfilter_graph_alloc_filter(graph, buffer_filt, name);
+    if (!ifp->filter) {
+        ret = AVERROR(ENOMEM);
         goto fail;
-    par->hw_frames_ctx = ifp->hw_frames_ctx;
+    }
+
+    par->format              = ifp->format;
+    par->time_base           = ifp->time_base;
+    par->frame_rate          = ifp->opts.framerate;
+    par->width               = ifp->width;
+    par->height              = ifp->height;
+    par->sample_aspect_ratio = ifp->sample_aspect_ratio.den > 0 ?
+                               ifp->sample_aspect_ratio : (AVRational){ 0, 1 };
+    par->color_space         = ifp->color_space;
+    par->color_range         = ifp->color_range;
+    par->hw_frames_ctx       = ifp->hw_frames_ctx;
     ret = av_buffersrc_parameters_set(ifp->filter, par);
     if (ret < 0)
         goto fail;
     av_freep(&par);
+
+    ret = avfilter_init_dict(ifp->filter, NULL);
+    if (ret < 0)
+        goto fail;
+
     last_filter = ifp->filter;
 
     desc = av_pix_fmt_desc_get(ifp->format);