diff mbox series

[FFmpeg-devel,12/21] fftools/ffmpeg_filter: reject filtergraphs with zero outputs

Message ID 20230614164908.28712-12-anton@khirnov.net
State Accepted
Commit b1a213ab5d84085ae2b98452daa8feda01881cbf
Headers show
Series [FFmpeg-devel,01/21] fftools/ffmpeg_dec: drop always-0 InputStream.prev_sub.ret | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov June 14, 2023, 4:48 p.m. UTC 
Nothing useful can be done with them currently.
---
 fftools/ffmpeg_filter.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Paul B Mahol June 14, 2023, 4:53 p.m. UTC | #1 
On Wed, Jun 14, 2023 at 6:51 PM Anton Khirnov <anton@khirnov.net> wrote:

> Nothing useful can be done with them currently.
> ---
>  fftools/ffmpeg_filter.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 4f7565e44e..54c7ed1f5c 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -845,6 +845,12 @@ FilterGraph *fg_create(char *graph_desc)
>          ofilter->name      = describe_filter_link(fg, cur, 0);
>      }
>
> +    if (!fg->nb_outputs) {
> +        av_log(fg, AV_LOG_FATAL, "A filtergraph has zero outputs, this is
> not supported\n");
> +        ret = AVERROR(ENOSYS);
> +        goto fail;
> +    }
> +
>  fail:
>      avfilter_inout_free(&inputs);
>      avfilter_inout_free(&outputs);
> --
> 2.40.1
>
>

NAK

This is functionality breaking change.

And why something that is currently supported should be not supported any
more.

That logic is completely flawed.


> _______________________________________________
> 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 14, 2023, 5 p.m. UTC | #2 
Quoting Paul B Mahol (2023-06-14 18:53:52)
> On Wed, Jun 14, 2023 at 6:51 PM Anton Khirnov <anton@khirnov.net> wrote:
> 
> > Nothing useful can be done with them currently.
> > ---
> >  fftools/ffmpeg_filter.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index 4f7565e44e..54c7ed1f5c 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -845,6 +845,12 @@ FilterGraph *fg_create(char *graph_desc)
> >          ofilter->name      = describe_filter_link(fg, cur, 0);
> >      }
> >
> > +    if (!fg->nb_outputs) {
> > +        av_log(fg, AV_LOG_FATAL, "A filtergraph has zero outputs, this is
> > not supported\n");
> > +        ret = AVERROR(ENOSYS);
> > +        goto fail;
> > +    }
> > +
> >  fail:
> >      avfilter_inout_free(&inputs);
> >      avfilter_inout_free(&outputs);
> > --
> > 2.40.1
> >
> >
> 
> NAK
> 
> This is functionality breaking change.
> 

Exactly what useful functionality is broken by this?

> And why something that is currently supported should be not supported any
> more.

It is not really supported. The scheduling logic is based around output
streams and cannot work without them.
Paul B Mahol June 14, 2023, 5:11 p.m. UTC | #3 
On Wed, Jun 14, 2023 at 7:01 PM Anton Khirnov <anton@khirnov.net> wrote:

> Quoting Paul B Mahol (2023-06-14 18:53:52)
> > On Wed, Jun 14, 2023 at 6:51 PM Anton Khirnov <anton@khirnov.net> wrote:
> >
> > > Nothing useful can be done with them currently.
> > > ---
> > >  fftools/ffmpeg_filter.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > > index 4f7565e44e..54c7ed1f5c 100644
> > > --- a/fftools/ffmpeg_filter.c
> > > +++ b/fftools/ffmpeg_filter.c
> > > @@ -845,6 +845,12 @@ FilterGraph *fg_create(char *graph_desc)
> > >          ofilter->name      = describe_filter_link(fg, cur, 0);
> > >      }
> > >
> > > +    if (!fg->nb_outputs) {
> > > +        av_log(fg, AV_LOG_FATAL, "A filtergraph has zero outputs,
> this is
> > > not supported\n");
> > > +        ret = AVERROR(ENOSYS);
> > > +        goto fail;
> > > +    }
> > > +
> > >  fail:
> > >      avfilter_inout_free(&inputs);
> > >      avfilter_inout_free(&outputs);
> > > --
> > > 2.40.1
> > >
> > >
> >
> > NAK
> >
> > This is functionality breaking change.
> >
>
> Exactly what useful functionality is broken by this?
>
> > And why something that is currently supported should be not supported any
> > more.
>
> It is not really supported. The scheduling logic is based around output
> streams and cannot work without them.
>

This works currently:

ffmpeg -i tom.wav -lavfi astats,anullsink -f null -


> --
> Anton Khirnov
> _______________________________________________
> 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 14, 2023, 5:14 p.m. UTC | #4 
Quoting Paul B Mahol (2023-06-14 19:11:35)
> On Wed, Jun 14, 2023 at 7:01 PM Anton Khirnov <anton@khirnov.net> wrote:
> 
> > Quoting Paul B Mahol (2023-06-14 18:53:52)
> > > On Wed, Jun 14, 2023 at 6:51 PM Anton Khirnov <anton@khirnov.net> wrote:
> > >
> > > > Nothing useful can be done with them currently.
> > > > ---
> > > >  fftools/ffmpeg_filter.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > > > index 4f7565e44e..54c7ed1f5c 100644
> > > > --- a/fftools/ffmpeg_filter.c
> > > > +++ b/fftools/ffmpeg_filter.c
> > > > @@ -845,6 +845,12 @@ FilterGraph *fg_create(char *graph_desc)
> > > >          ofilter->name      = describe_filter_link(fg, cur, 0);
> > > >      }
> > > >
> > > > +    if (!fg->nb_outputs) {
> > > > +        av_log(fg, AV_LOG_FATAL, "A filtergraph has zero outputs,
> > this is
> > > > not supported\n");
> > > > +        ret = AVERROR(ENOSYS);
> > > > +        goto fail;
> > > > +    }
> > > > +
> > > >  fail:
> > > >      avfilter_inout_free(&inputs);
> > > >      avfilter_inout_free(&outputs);
> > > > --
> > > > 2.40.1
> > > >
> > > >
> > >
> > > NAK
> > >
> > > This is functionality breaking change.
> > >
> >
> > Exactly what useful functionality is broken by this?
> >
> > > And why something that is currently supported should be not supported any
> > > more.
> >
> > It is not really supported. The scheduling logic is based around output
> > streams and cannot work without them.
> >
> 
> This works currently:
> 
> ffmpeg -i tom.wav -lavfi astats,anullsink -f null -

And what useful functionality is provided by this that you cannot
achieve otherwise?
diff mbox series

Patch

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 4f7565e44e..54c7ed1f5c 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -845,6 +845,12 @@  FilterGraph *fg_create(char *graph_desc)
         ofilter->name      = describe_filter_link(fg, cur, 0);
     }
 
+    if (!fg->nb_outputs) {
+        av_log(fg, AV_LOG_FATAL, "A filtergraph has zero outputs, this is not supported\n");
+        ret = AVERROR(ENOSYS);
+        goto fail;
+    }
+
 fail:
     avfilter_inout_free(&inputs);
     avfilter_inout_free(&outputs);