diff mbox series

[FFmpeg-devel,4/5] lavfi/graphparser: reimplement avfilter_graph_parse* using new API

Message ID 20230120193132.21597-4-anton@khirnov.net
State Accepted
Commit f17051eaae278c6683492c037e5a1658ace64bec
Headers show
Series [FFmpeg-devel,1/5] lavfi/avfilter: export process_options() | expand

Checks

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

Commit Message

Anton Khirnov Jan. 20, 2023, 7:31 p.m. UTC
---
 libavfilter/graphparser.c | 537 +++++++++-----------------------------
 1 file changed, 126 insertions(+), 411 deletions(-)

Comments

Nicolas George Jan. 20, 2023, 8:47 p.m. UTC | #1
Anton Khirnov (12023-01-20):
> ---
>  libavfilter/graphparser.c | 537 +++++++++-----------------------------
>  1 file changed, 126 insertions(+), 411 deletions(-)

I notice you removed the "RFC" from the subject line, but I probably
have comments. I need some time, my time is very limited these days.

By the way, I am baffled that you would write a 30k+ patch on code you
do not maintain and do not know well without consulting the maintainer
who knows it.

Regards,
Jean-Baptiste Kempf Jan. 20, 2023, 9:11 p.m. UTC | #2
On Fri, 20 Jan 2023, at 21:47, Nicolas George wrote:
> have comments. I need some time, my time is very limited these days.
>
> By the way, I am baffled that you would write a 30k+ patch on code you
> do not maintain and do not know well without consulting the maintainer
> who knows it.

First, maintainer does not mean owner, so of course, it's normal that people work on what they want, without consulting before. Maintainer can reject with explanations, of course, but that does not forbid anyone to work on what they want...

Then, also, in the same email you say  "I have no time" and "you should consult with me before". Those are contradictory.

jb
Nicolas George Jan. 20, 2023, 9:32 p.m. UTC | #3
Jean-Baptiste Kempf (12023-01-20):
> First, maintainer does not mean owner

No. But maintainer means person who knows the code very well.

> Then, also, in the same email you say  "I have no time" and "you should consult with me before". Those are contradictory.

This is not contradictory. I have no time to reply in days, but this
patch took more than days in the making, I would have ample chance to
take the time.

Writing this patch alone, Anton, who has contributed to libavfilter a
lot at this time, incurs the risk of neglecting issues that might
require rewriting a significant part of it, or even voiding it entirely.
Somebody who knows the code and has given a lot of thought to future
developments (me for example) could have pointed to those possible
issues early, saving a lot of time to Anton.

This is not specific to this issue: Why would ANYBODY spend a lot of
time and effort writing a far-reaching patch to a part of the code they
do not know without consulting their fellow developers who know this
part better, the maintainer in particular?

I insist, it is a very specific combinations of criteria:

- Writing a small patch for a part you do not know well without
  consulting is fine, because the possible wasted effort is small and
  the likelihood of severe mistakes is small. It is quadratic.

- Writing a large patch for a part of the code you know well or for a
  new feature is fine, of course.

But the combination (1) code that you do not know well, (2) patch that
took effort writing, (3) patch that goes far and this is at risk of hard
to see issues, then not consulting with people who know the code best
seems really absurd.

For lack of a better explanation, I perceive cases like that when they
happen as a lack of team-spirit that is detrimental to this project.

I mean, it would not have taken a lot of time: "Hey guys, I am
considering redesigning the graph parsing API in lavfi. Here are my main
ideas: ... Do you have comments before I really start?", and it could
have made the result a lot better from the get go.

Also, it would have been the POLITE thing to do. It would have been the
thing to do if one considers me as an equal.
Nicolas George Jan. 20, 2023, 9:39 p.m. UTC | #4
Nicolas George (12023-01-20):
> - Writing a large patch for a part of the code you know well or for a
>   new feature is fine, of course.

In fact, even for code we know well, we should consult, as long as we
are not the only one who knows it well: as long as other developers
might also have plans that we would be interfering with, sharing intents
early is the only correct thing to do towards our peers.
Anton Khirnov Jan. 20, 2023, 10:36 p.m. UTC | #5
Quoting Nicolas George (2023-01-20 21:47:16)
> Anton Khirnov (12023-01-20):
> > ---
> >  libavfilter/graphparser.c | 537 +++++++++-----------------------------
> >  1 file changed, 126 insertions(+), 411 deletions(-)
> 
> I notice you removed the "RFC" from the subject line,

That means I now consider the code to be ready for a full review
rather than just comments on the overall approach.

> By the way, I am baffled that you would write a 30k+ patch on code you
> do not maintain

Does the graphparser have a maintainer? I did not notice.
$ grep graphparser MAINTAINERS
$

> and do not know well without consulting the maintainer
> who knows it.

Maybe it's the person who authored the most commits touching that file
recently
$ git shortlog -ns --since=2012 --no-merges origin/master libavfilter/graphparser.c
 16  Anton Khirnov
  8  Michael Niedermayer
  4  Andreas Rheinhardt

Or maybe we should consider avfilter.h too, since I'm adding new API
$ git shortlog -ns --since=2012 --no-merges origin/master libavfilter/graphparser.c  libavfilter/avfilter.h
 73  Anton Khirnov
 32  Nicolas George
 23  Stefano Sabatini

Hmm, now who could possibly be the person who knows that code? Any
guesses?
James Almer Jan. 23, 2023, 12:29 p.m. UTC | #6
On 1/20/2023 6:39 PM, Nicolas George wrote:
> Nicolas George (12023-01-20):
>> - Writing a large patch for a part of the code you know well or for a
>>    new feature is fine, of course.
> 
> In fact, even for code we know well, we should consult, as long as we
> are not the only one who knows it well: as long as other developers
> might also have plans that we would be interfering with, sharing intents
> early is the only correct thing to do towards our peers.

Nobody is disrespecting you for writing a big patch that implements or 
rewrites some API from a part of the code you consider yourself 
knowledgeable about without consulting you personally beforehand. He in 
fact sent a Request for Comment initial set precisely because he wanted 
input on the overall approach before sending a version that's ready for 
a proper review.
What's the difference between a RFC and a "I have this idea, what do you 
think of this mock up?" email? That if suggestions are made to improve 
or change said approach, then at worst what happens is that the writer 
wasted /his/ time writing 30k lines of which a fifth might need to be 
redone.
In all cases, you lost nothing, and the author at worst lost time. I 
have no idea how you come to the conclusion that you were disrespected, 
or not considered a peer.
Paul B Mahol Jan. 24, 2023, 9:51 p.m. UTC | #7
On 1/20/23, Nicolas George <george@nsup.org> wrote:
> Nicolas George (12023-01-20):
>> - Writing a large patch for a part of the code you know well or for a
>>   new feature is fine, of course.
>
> In fact, even for code we know well, we should consult, as long as we
> are not the only one who knows it well: as long as other developers
> might also have plans that we would be interfering with, sharing intents
> early is the only correct thing to do towards our peers.
>


Please be polite, technical and respectful to contributors, thanks.
Nicolas George Jan. 30, 2023, 1:04 p.m. UTC | #8
James Almer (12023-01-23):
> What's the difference between a RFC and a "I have this idea, what do you
> think of this mock up?" email? That if suggestions are made to improve or
> change said approach, then at worst what happens is that the writer wasted
> /his/ time writing 30k lines of which a fifth might need to be redone.

You are assuming that somebody who already wrote 30K of code (not even
lines) will be as ready to make changes that require rewriting 27K than
somebody who did not already write the code.

We had the exact problem with channel layouts: the structure should have
been refcounted, but since the code was written without consulting
people who would have to use it and neglected necessary features, it was
not done, and since the code was already written the change was not
seriously considered.

Furthermore, communicating early would save time to somebody who might
be at the early stages of working on the same thing.

Regards,
Marton Balint May 30, 2023, 9:09 p.m. UTC | #9
On Fri, 20 Jan 2023, Anton Khirnov wrote:

> ---
> libavfilter/graphparser.c | 537 +++++++++-----------------------------
> 1 file changed, 126 insertions(+), 411 deletions(-)

This change makes error messages for parse failures less useful or 
completely missing. E.g.:

ffmpeg -f lavfi -i testsrc -vf yadif,scale=dummy=boo -f null none

Before:

[Parsed_scale_0 @ 0x555c19335b00] Option 'dummy' not found
[AVFilterGraph @ 0x555c19333e00] Error initializing filter 'scale' with args 'dummy=boo:flags=bicubic'
Error reinitializing filters!

After:

[AVFilterGraph @ 0x3c5a100] Error applying filter options
Error reinitializing filters!


ffplay -f lavfi -i testsrc -vf yadif,scale=dummy=boo

Before:

[Parsed_scale_1 @ 0x55fa2b935080] Option 'dummy' not found
Error initializing filter 'scale' with args 'dummy=boo:flags=bicubic'

After:

(no error message)

Regards,
Marton
Paul B Mahol May 31, 2023, 2:39 p.m. UTC | #10
On Tue, May 30, 2023 at 11:10 PM Marton Balint <cus@passwd.hu> wrote:

>
>
> On Fri, 20 Jan 2023, Anton Khirnov wrote:
>
> > ---
> > libavfilter/graphparser.c | 537 +++++++++-----------------------------
> > 1 file changed, 126 insertions(+), 411 deletions(-)
>
> This change makes error messages for parse failures less useful or
> completely missing. E.g.:
>
> ffmpeg -f lavfi -i testsrc -vf yadif,scale=dummy=boo -f null none
>
> Before:
>
> [Parsed_scale_0 @ 0x555c19335b00] Option 'dummy' not found
> [AVFilterGraph @ 0x555c19333e00] Error initializing filter 'scale' with
> args 'dummy=boo:flags=bicubic'
> Error reinitializing filters!
>
> After:
>
> [AVFilterGraph @ 0x3c5a100] Error applying filter options
> Error reinitializing filters!
>
>
> ffplay -f lavfi -i testsrc -vf yadif,scale=dummy=boo
>
> Before:
>
> [Parsed_scale_1 @ 0x55fa2b935080] Option 'dummy' not found
> Error initializing filter 'scale' with args 'dummy=boo:flags=bicubic'
>
> After:
>
> (no error message)
>

Yes, I was hit by this bug, multiple times recently.
It makes my pain bigger.


> Regards,
> Marton
> _______________________________________________
> 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".
>
Anton Khirnov June 3, 2023, 10:02 a.m. UTC | #11
Quoting Marton Balint (2023-05-30 23:09:50)
> 
> 
> On Fri, 20 Jan 2023, Anton Khirnov wrote:
> 
> > ---
> > libavfilter/graphparser.c | 537 +++++++++-----------------------------
> > 1 file changed, 126 insertions(+), 411 deletions(-)
> 
> This change makes error messages for parse failures less useful or 
> completely missing. E.g.:
> 
> ffmpeg -f lavfi -i testsrc -vf yadif,scale=dummy=boo -f null none
> 
> Before:
> 
> [Parsed_scale_0 @ 0x555c19335b00] Option 'dummy' not found
> [AVFilterGraph @ 0x555c19333e00] Error initializing filter 'scale' with args 'dummy=boo:flags=bicubic'
> Error reinitializing filters!
> 
> After:
> 
> [AVFilterGraph @ 0x3c5a100] Error applying filter options
> Error reinitializing filters!

Cannot reproduce, for me current git master says:
Input #0, lavfi, from 'testsrc':
  Duration: N/A, start: 0.000000, bitrate: N/A
  Stream #0:0: Video: wrapped_avframe, rgb24, 320x240 [SAR 1:1 DAR 4:3], 25 fps, 25 tbr, 25 tbn
Error applying option 'dummy' to filter 'scale': Option not found
Option not found

Which could be improved, but it does mention the unknown option name.
> ffplay -f lavfi -i testsrc -vf yadif,scale=dummy=boo
> 
> Before:
> 
> [Parsed_scale_1 @ 0x55fa2b935080] Option 'dummy' not found
> Error initializing filter 'scale' with args 'dummy=boo:flags=bicubic'
> 
> After:
> 
> (no error message)

Should be addressed by the patch I just sent.
diff mbox series

Patch

diff --git a/libavfilter/graphparser.c b/libavfilter/graphparser.c
index ac6171e0c4..5db8b95a0c 100644
--- a/libavfilter/graphparser.c
+++ b/libavfilter/graphparser.c
@@ -33,26 +33,6 @@ 
 
 #define WHITESPACES " \n\t\r"
 
-/**
- * Link two filters together.
- *
- * @see avfilter_link()
- */
-static int link_filter(AVFilterContext *src, int srcpad,
-                       AVFilterContext *dst, int dstpad,
-                       void *log_ctx)
-{
-    int ret;
-    if ((ret = avfilter_link(src, srcpad, dst, dstpad))) {
-        av_log(log_ctx, AV_LOG_ERROR,
-               "Cannot create the link %s:%d -> %s:%d\n",
-               src->filter->name, srcpad, dst->filter->name, dstpad);
-        return ret;
-    }
-
-    return 0;
-}
-
 /**
  * Parse the name of a link, which has the format "[linkname]".
  *
@@ -87,119 +67,6 @@  static char *parse_link_name(const char **buf, void *log_ctx)
     return name;
 }
 
-/**
- * Create an instance of a filter, initialize and insert it in the
- * filtergraph in *ctx.
- *
- * @param filt_ctx put here a filter context in case of successful creation and configuration, NULL otherwise.
- * @param ctx the filtergraph context
- * @param index an index which is supposed to be unique for each filter instance added to the filtergraph
- * @param name the name of the filter to create, can be filter name or filter_name\@id as instance name
- * @param args the arguments provided to the filter during its initialization
- * @param log_ctx the log context to use
- * @return >= 0 in case of success, a negative AVERROR code otherwise
- */
-static int create_filter(AVFilterContext **filt_ctx, AVFilterGraph *ctx, int index,
-                         const char *name, const char *args, void *log_ctx)
-{
-    const AVFilter *filt;
-    char name2[30];
-    const char *inst_name = NULL, *filt_name = NULL;
-    int ret, k;
-
-    av_strlcpy(name2, name, sizeof(name2));
-
-    for (k = 0; name2[k]; k++) {
-        if (name2[k] == '@' && name[k+1]) {
-            name2[k] = 0;
-            inst_name = name;
-            filt_name = name2;
-            break;
-        }
-    }
-
-    if (!inst_name) {
-        snprintf(name2, sizeof(name2), "Parsed_%s_%d", name, index);
-        inst_name = name2;
-        filt_name = name;
-    }
-
-    filt = avfilter_get_by_name(filt_name);
-
-    if (!filt) {
-        av_log(log_ctx, AV_LOG_ERROR,
-               "No such filter: '%s'\n", filt_name);
-        return AVERROR(EINVAL);
-    }
-
-    *filt_ctx = avfilter_graph_alloc_filter(ctx, filt, inst_name);
-    if (!*filt_ctx) {
-        av_log(log_ctx, AV_LOG_ERROR,
-               "Error creating filter '%s'\n", filt_name);
-        return AVERROR(ENOMEM);
-    }
-
-    if (!strcmp(filt_name, "scale") && ctx->scale_sws_opts) {
-        ret = av_set_options_string(*filt_ctx, ctx->scale_sws_opts, "=", ":");
-        if (ret < 0)
-            return ret;
-    }
-
-    ret = avfilter_init_str(*filt_ctx, args);
-    if (ret < 0) {
-        av_log(log_ctx, AV_LOG_ERROR,
-               "Error initializing filter '%s'", filt_name);
-        if (args)
-            av_log(log_ctx, AV_LOG_ERROR, " with args '%s'", args);
-        av_log(log_ctx, AV_LOG_ERROR, "\n");
-        avfilter_free(*filt_ctx);
-        *filt_ctx = NULL;
-    }
-
-    return ret;
-}
-
-/**
- * Parse a string of the form FILTER_NAME[=PARAMS], and create a
- * corresponding filter instance which is added to graph with
- * create_filter().
- *
- * @param filt_ctx Pointer that is set to the created and configured filter
- *                 context on success, set to NULL on failure.
- * @param filt_ctx put here a pointer to the created filter context on
- * success, NULL otherwise
- * @param buf pointer to the buffer to parse, *buf will be updated to
- * point to the char next after the parsed string
- * @param index an index which is assigned to the created filter
- * instance, and which is supposed to be unique for each filter
- * instance added to the filtergraph
- * @return >= 0 in case of success, a negative AVERROR code otherwise
- */
-static int parse_filter(AVFilterContext **filt_ctx, const char **buf, AVFilterGraph *graph,
-                        int index, void *log_ctx)
-{
-    char *opts = NULL;
-    char *name = av_get_token(buf, "=,;[");
-    int ret;
-
-    if (!name)
-        return AVERROR(ENOMEM);
-
-    if (**buf == '=') {
-        (*buf)++;
-        opts = av_get_token(buf, "[],;");
-        if (!opts) {
-            av_free(name);
-            return AVERROR(ENOMEM);
-        }
-    }
-
-    ret = create_filter(filt_ctx, graph, index, name, opts, log_ctx);
-    av_free(name);
-    av_free(opts);
-    return ret;
-}
-
 AVFilterInOut *avfilter_inout_alloc(void)
 {
     return av_mallocz(sizeof(AVFilterInOut));
@@ -232,12 +99,6 @@  static AVFilterInOut *extract_inout(const char *label, AVFilterInOut **links)
     return ret;
 }
 
-static void insert_inout(AVFilterInOut **inouts, AVFilterInOut *element)
-{
-    element->next = *inouts;
-    *inouts = element;
-}
-
 static void append_inout(AVFilterInOut **inouts, AVFilterInOut **element)
 {
     while (*inouts && (*inouts)->next)
@@ -250,144 +111,6 @@  static void append_inout(AVFilterInOut **inouts, AVFilterInOut **element)
     *element = NULL;
 }
 
-static int link_filter_inouts(AVFilterContext *filt_ctx,
-                              AVFilterInOut **curr_inputs,
-                              AVFilterInOut **open_inputs, void *log_ctx)
-{
-    int pad, ret;
-
-    for (pad = 0; pad < filt_ctx->nb_inputs; pad++) {
-        AVFilterInOut *p = *curr_inputs;
-
-        if (p) {
-            *curr_inputs = (*curr_inputs)->next;
-            p->next = NULL;
-        } else if (!(p = av_mallocz(sizeof(*p))))
-            return AVERROR(ENOMEM);
-
-        if (p->filter_ctx) {
-            ret = link_filter(p->filter_ctx, p->pad_idx, filt_ctx, pad, log_ctx);
-            av_freep(&p->name);
-            av_freep(&p);
-            if (ret < 0)
-                return ret;
-        } else {
-            p->filter_ctx = filt_ctx;
-            p->pad_idx = pad;
-            append_inout(open_inputs, &p);
-        }
-    }
-
-    if (*curr_inputs) {
-        av_log(log_ctx, AV_LOG_ERROR,
-               "Too many inputs specified for the \"%s\" filter.\n",
-               filt_ctx->filter->name);
-        return AVERROR(EINVAL);
-    }
-
-    pad = filt_ctx->nb_outputs;
-    while (pad--) {
-        AVFilterInOut *currlinkn = av_mallocz(sizeof(AVFilterInOut));
-        if (!currlinkn)
-            return AVERROR(ENOMEM);
-        currlinkn->filter_ctx  = filt_ctx;
-        currlinkn->pad_idx = pad;
-        insert_inout(curr_inputs, currlinkn);
-    }
-
-    return 0;
-}
-
-static int parse_inputs(const char **buf, AVFilterInOut **curr_inputs,
-                        AVFilterInOut **open_outputs, void *log_ctx)
-{
-    AVFilterInOut *parsed_inputs = NULL;
-    int pad = 0;
-
-    while (**buf == '[') {
-        char *name = parse_link_name(buf, log_ctx);
-        AVFilterInOut *match;
-
-        if (!name) {
-            avfilter_inout_free(&parsed_inputs);
-            return AVERROR(EINVAL);
-        }
-
-        /* First check if the label is not in the open_outputs list */
-        match = extract_inout(name, open_outputs);
-
-        if (match) {
-            av_free(name);
-        } else {
-            /* Not in the list, so add it as an input */
-            if (!(match = av_mallocz(sizeof(AVFilterInOut)))) {
-                avfilter_inout_free(&parsed_inputs);
-                av_free(name);
-                return AVERROR(ENOMEM);
-            }
-            match->name    = name;
-            match->pad_idx = pad;
-        }
-
-        append_inout(&parsed_inputs, &match);
-
-        *buf += strspn(*buf, WHITESPACES);
-        pad++;
-    }
-
-    append_inout(&parsed_inputs, curr_inputs);
-    *curr_inputs = parsed_inputs;
-
-    return pad;
-}
-
-static int parse_outputs(const char **buf, AVFilterInOut **curr_inputs,
-                         AVFilterInOut **open_inputs,
-                         AVFilterInOut **open_outputs, void *log_ctx)
-{
-    int ret, pad = 0;
-
-    while (**buf == '[') {
-        char *name = parse_link_name(buf, log_ctx);
-        AVFilterInOut *match;
-
-        AVFilterInOut *input = *curr_inputs;
-
-        if (!name)
-            return AVERROR(EINVAL);
-
-        if (!input) {
-            av_log(log_ctx, AV_LOG_ERROR,
-                   "No output pad can be associated to link label '%s'.\n", name);
-            av_free(name);
-            return AVERROR(EINVAL);
-        }
-        *curr_inputs = (*curr_inputs)->next;
-
-        /* First check if the label is not in the open_inputs list */
-        match = extract_inout(name, open_inputs);
-
-        if (match) {
-            ret = link_filter(input->filter_ctx, input->pad_idx,
-                              match->filter_ctx, match->pad_idx, log_ctx);
-            av_freep(&match->name);
-            av_freep(&name);
-            av_freep(&match);
-            av_freep(&input);
-            if (ret < 0)
-                return ret;
-        } else {
-            /* Not in the list, so add the first input as an open_output */
-            input->name = name;
-            insert_inout(open_outputs, input);
-        }
-        *buf += strspn(*buf, WHITESPACES);
-        pad++;
-    }
-
-    return pad;
-}
-
 static int parse_sws_flags(const char **buf, char **dst, void *log_ctx)
 {
     char *p = strchr(*buf, ';');
@@ -415,66 +138,24 @@  int avfilter_graph_parse2(AVFilterGraph *graph, const char *filters,
                           AVFilterInOut **inputs,
                           AVFilterInOut **outputs)
 {
-    int index = 0, ret = 0;
-    char chr = 0;
-
-    AVFilterInOut *curr_inputs = NULL, *open_inputs = NULL, *open_outputs = NULL;
-
-    filters += strspn(filters, WHITESPACES);
-
-    if ((ret = parse_sws_flags(&filters, &graph->scale_sws_opts, graph)) < 0)
-        goto end;
-
-    do {
-        AVFilterContext *filter;
-        filters += strspn(filters, WHITESPACES);
-
-        if ((ret = parse_inputs(&filters, &curr_inputs, &open_outputs, graph)) < 0)
-            goto end;
-        if ((ret = parse_filter(&filter, &filters, graph, index, graph)) < 0)
-            goto end;
-
-
-        if ((ret = link_filter_inouts(filter, &curr_inputs, &open_inputs, graph)) < 0)
-            goto end;
-
-        if ((ret = parse_outputs(&filters, &curr_inputs, &open_inputs, &open_outputs,
-                                 graph)) < 0)
-            goto end;
-
-        filters += strspn(filters, WHITESPACES);
-        chr = *filters++;
+    AVFilterGraphSegment *seg;
+    int ret;
 
-        if (chr == ';' && curr_inputs)
-            append_inout(&open_outputs, &curr_inputs);
-        index++;
-    } while (chr == ',' || chr == ';');
+    ret = avfilter_graph_segment_parse(graph, filters, 0, &seg);
+    if (ret < 0)
+        return ret;
 
-    if (chr) {
-        av_log(graph, AV_LOG_ERROR,
-               "Unable to parse graph description substring: \"%s\"\n",
-               filters - 1);
-        ret = AVERROR(EINVAL);
+    ret = avfilter_graph_segment_apply(seg, 0, inputs, outputs);
+    avfilter_graph_segment_free(&seg);
+    if (ret < 0)
         goto end;
-    }
-
-    append_inout(&open_outputs, &curr_inputs);
-
 
-    *inputs  = open_inputs;
-    *outputs = open_outputs;
     return 0;
 
 end:
     while (graph->nb_filters)
         avfilter_free(graph->filters[0]);
     av_freep(&graph->filters);
-    avfilter_inout_free(&open_inputs);
-    avfilter_inout_free(&open_outputs);
-    avfilter_inout_free(&curr_inputs);
-
-    *inputs  = NULL;
-    *outputs = NULL;
 
     return ret;
 }
@@ -542,90 +223,6 @@  int avfilter_graph_parse(AVFilterGraph *graph, const char *filters,
     return ret;
 }
 
-int avfilter_graph_parse_ptr(AVFilterGraph *graph, const char *filters,
-                         AVFilterInOut **open_inputs_ptr, AVFilterInOut **open_outputs_ptr,
-                         void *log_ctx)
-{
-    int index = 0, ret = 0;
-    char chr = 0;
-
-    AVFilterInOut *curr_inputs = NULL;
-    AVFilterInOut *open_inputs  = open_inputs_ptr  ? *open_inputs_ptr  : NULL;
-    AVFilterInOut *open_outputs = open_outputs_ptr ? *open_outputs_ptr : NULL;
-
-    if ((ret = parse_sws_flags(&filters, &graph->scale_sws_opts, graph)) < 0)
-        goto end;
-
-    do {
-        AVFilterContext *filter;
-        const char *filterchain = filters;
-        filters += strspn(filters, WHITESPACES);
-
-        if ((ret = parse_inputs(&filters, &curr_inputs, &open_outputs, log_ctx)) < 0)
-            goto end;
-
-        if ((ret = parse_filter(&filter, &filters, graph, index, log_ctx)) < 0)
-            goto end;
-
-        if (filter->nb_inputs == 1 && !curr_inputs && !index) {
-            /* First input pad, assume it is "[in]" if not specified */
-            const char *tmp = "[in]";
-            if ((ret = parse_inputs(&tmp, &curr_inputs, &open_outputs, log_ctx)) < 0)
-                goto end;
-        }
-
-        if ((ret = link_filter_inouts(filter, &curr_inputs, &open_inputs, log_ctx)) < 0)
-            goto end;
-
-        if ((ret = parse_outputs(&filters, &curr_inputs, &open_inputs, &open_outputs,
-                                 log_ctx)) < 0)
-            goto end;
-
-        filters += strspn(filters, WHITESPACES);
-        chr = *filters++;
-
-        if (chr == ';' && curr_inputs) {
-            av_log(log_ctx, AV_LOG_ERROR,
-                   "Invalid filterchain containing an unlabelled output pad: \"%s\"\n",
-                   filterchain);
-            ret = AVERROR(EINVAL);
-            goto end;
-        }
-        index++;
-    } while (chr == ',' || chr == ';');
-
-    if (chr) {
-        av_log(log_ctx, AV_LOG_ERROR,
-               "Unable to parse graph description substring: \"%s\"\n",
-               filters - 1);
-        ret = AVERROR(EINVAL);
-        goto end;
-    }
-
-    if (curr_inputs) {
-        /* Last output pad, assume it is "[out]" if not specified */
-        const char *tmp = "[out]";
-        if ((ret = parse_outputs(&tmp, &curr_inputs, &open_inputs, &open_outputs,
-                                 log_ctx)) < 0)
-            goto end;
-    }
-
-end:
-    /* clear open_in/outputs only if not passed as parameters */
-    if (open_inputs_ptr) *open_inputs_ptr = open_inputs;
-    else avfilter_inout_free(&open_inputs);
-    if (open_outputs_ptr) *open_outputs_ptr = open_outputs;
-    else avfilter_inout_free(&open_outputs);
-    avfilter_inout_free(&curr_inputs);
-
-    if (ret < 0) {
-        while (graph->nb_filters)
-            avfilter_free(graph->filters[0]);
-        av_freep(&graph->filters);
-    }
-    return ret;
-}
-
 static void pad_params_free(AVFilterPadParams **pfpp)
 {
     AVFilterPadParams *fpp = *pfpp;
@@ -1298,3 +895,121 @@  int avfilter_graph_segment_apply(AVFilterGraphSegment *seg, int flags,
 
     return 0;
 }
+
+int avfilter_graph_parse_ptr(AVFilterGraph *graph, const char *filters,
+                         AVFilterInOut **open_inputs_ptr, AVFilterInOut **open_outputs_ptr,
+                         void *log_ctx)
+{
+    AVFilterInOut *user_inputs  = open_inputs_ptr  ? *open_inputs_ptr  : NULL;
+    AVFilterInOut *user_outputs = open_outputs_ptr ? *open_outputs_ptr : NULL;
+
+    AVFilterInOut *inputs = NULL, *outputs = NULL;
+    AVFilterGraphSegment *seg = NULL;
+    AVFilterChain         *ch;
+    AVFilterParams         *p;
+    int ret;
+
+    ret = avfilter_graph_segment_parse(graph, filters, 0, &seg);
+    if (ret < 0)
+        goto end;
+
+    ret = avfilter_graph_segment_create_filters(seg, 0);
+    if (ret < 0)
+        goto end;
+
+    ret = avfilter_graph_segment_apply_opts(seg, 0);
+    if (ret < 0)
+        goto end;
+
+    ret = avfilter_graph_segment_init(seg, 0);
+    if (ret < 0)
+        goto end;
+
+    /* First input pad, assume it is "[in]" if not specified */
+    p = seg->chains[0]->filters[0];
+    if (p->filter->nb_inputs == 1 && !p->inputs) {
+        const char *tmp = "[in]";
+
+        ret = linklabels_parse(graph, &tmp, &p->inputs, &p->nb_inputs);
+        if (ret < 0)
+            goto end;
+    }
+
+    /* Last output pad, assume it is "[out]" if not specified */
+    ch = seg->chains[seg->nb_chains - 1];
+    p = ch->filters[ch->nb_filters - 1];
+    if (p->filter->nb_outputs == 1 && !p->outputs) {
+        const char *tmp = "[out]";
+
+        ret = linklabels_parse(graph, &tmp, &p->outputs, &p->nb_outputs);
+        if (ret < 0)
+            goto end;
+    }
+
+    ret = avfilter_graph_segment_apply(seg, 0, &inputs, &outputs);
+    avfilter_graph_segment_free(&seg);
+    if (ret < 0)
+        goto end;
+
+    // process user-supplied inputs/outputs
+    while (inputs) {
+        AVFilterInOut *cur, *match = NULL;
+
+        cur       = inputs;
+        inputs    = cur->next;
+        cur->next = NULL;
+
+        if (cur->name)
+            match = extract_inout(cur->name, &user_outputs);
+
+        if (match) {
+            ret = avfilter_link(match->filter_ctx, match->pad_idx,
+                                cur->filter_ctx, cur->pad_idx);
+            avfilter_inout_free(&match);
+            avfilter_inout_free(&cur);
+            if (ret < 0)
+                goto end;
+        } else
+            append_inout(&user_inputs, &cur);
+    }
+    while (outputs) {
+        AVFilterInOut *cur, *match = NULL;
+
+        cur       = outputs;
+        outputs   = cur->next;
+        cur->next = NULL;
+
+        if (cur->name)
+            match = extract_inout(cur->name, &user_inputs);
+
+        if (match) {
+            ret = avfilter_link(cur->filter_ctx, cur->pad_idx,
+                                match->filter_ctx, match->pad_idx);
+            avfilter_inout_free(&match);
+            avfilter_inout_free(&cur);
+            if (ret < 0)
+                goto end;
+        } else
+            append_inout(&user_outputs, &cur);
+    }
+
+end:
+    avfilter_graph_segment_free(&seg);
+
+    if (ret < 0) {
+        while (graph->nb_filters)
+            avfilter_free(graph->filters[0]);
+        av_freep(&graph->filters);
+    }
+
+    /* clear open_in/outputs only if not passed as parameters */
+    if (open_inputs_ptr) *open_inputs_ptr = user_inputs;
+    else avfilter_inout_free(&user_inputs);
+    if (open_outputs_ptr) *open_outputs_ptr = user_outputs;
+    else avfilter_inout_free(&user_outputs);
+
+    avfilter_inout_free(&inputs);
+    avfilter_inout_free(&outputs);
+
+    return ret;
+}