diff mbox series

[FFmpeg-devel] avfilter/vf_frei0r: Copy to frame allocated according to frei0r requirements

Message ID 20220411133148.10292-1-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel] avfilter/vf_frei0r: Copy to frame allocated according to frei0r requirements | 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
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Michael Niedermayer April 11, 2022, 1:31 p.m. UTC
Fixes: issues with non trivial linesize

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavfilter/vf_frei0r.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

Comments

Paul B Mahol April 11, 2022, 5:34 p.m. UTC | #1
On Mon, Apr 11, 2022 at 3:32 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Fixes: issues with non trivial linesize
>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavfilter/vf_frei0r.c | 40 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> index 9cd0098e73..c9b698897f 100644
> --- a/libavfilter/vf_frei0r.c
> +++ b/libavfilter/vf_frei0r.c
> @@ -349,18 +349,41 @@ static int query_formats(AVFilterContext *ctx)
>      return ff_set_common_formats(ctx, formats);
>  }
>
> +static AVFrame *getframe(AVFilterLink *link)
> +{
> +    int ret;
> +    AVFrame *frame = av_frame_alloc();
> +    if (!frame)
> +        return NULL;
> +
> +    frame->width  = link->w;
> +    frame->height = link->h;
> +    frame->format = link->format;
> +    ret = av_frame_get_buffer(frame, 16);
> +    if (ret < 0)
> +        av_frame_free(&frame);
> +
> +    return frame;
> +}
> +
>  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  {
>      Frei0rContext *s = inlink->dst->priv;
>      AVFilterLink *outlink = inlink->dst->outputs[0];
> -    AVFrame *out;
> +    AVFrame *out = getframe(outlink);
> +    if (!out)
> +        goto fail;
>
> -    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> -    if (!out) {
> +    av_frame_copy_props(out, in);
> +
> +    if (in->linesize[0] != out->linesize[0]) {
> +        AVFrame *in2 = getframe(outlink);
> +        if (!in2)
> +            goto fail;
> +        av_frame_copy(in2, in);
>          av_frame_free(&in);
> -        return AVERROR(ENOMEM);
> +        in = in2;
>      }
> -    av_frame_copy_props(out, in);
>
>      s->update(s->instance, in->pts * av_q2d(inlink->time_base) * 1000,
>                     (const uint32_t *)in->data[0],
> @@ -369,6 +392,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>      av_frame_free(&in);
>
>      return ff_filter_frame(outlink, out);
> +fail:
> +    av_frame_free(&in);
> +    av_frame_free(&out);
> +    return AVERROR(ENOMEM);
>  }
>
>  static int process_command(AVFilterContext *ctx, const char *cmd, const
> char *args,
> @@ -473,9 +500,6 @@ static int source_request_frame(AVFilterLink *outlink)
>      frame->sample_aspect_ratio = (AVRational) {1, 1};
>      frame->pts = s->pts++;
>
> -    s->update(s->instance, av_rescale_q(frame->pts, s->time_base,
> (AVRational){1,1000}),
> -                   NULL, (uint32_t *)frame->data[0]);
>
>
Why this is removed?

That handle frei0r source filters.


>      return ff_filter_frame(outlink, frame);
>  }
>
> --
> 2.17.1
>
> _______________________________________________
> 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".
>
James Almer April 11, 2022, 6:01 p.m. UTC | #2
On 4/11/2022 10:31 AM, Michael Niedermayer wrote:
> Fixes: issues with non trivial linesize
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavfilter/vf_frei0r.c | 40 ++++++++++++++++++++++++++++++++--------
>   1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> index 9cd0098e73..c9b698897f 100644
> --- a/libavfilter/vf_frei0r.c
> +++ b/libavfilter/vf_frei0r.c
> @@ -349,18 +349,41 @@ static int query_formats(AVFilterContext *ctx)
>       return ff_set_common_formats(ctx, formats);
>   }
>   
> +static AVFrame *getframe(AVFilterLink *link)
> +{
> +    int ret;
> +    AVFrame *frame = av_frame_alloc();
> +    if (!frame)
> +        return NULL;
> +
> +    frame->width  = link->w;
> +    frame->height = link->h;
> +    frame->format = link->format;
> +    ret = av_frame_get_buffer(frame, 16);

Maybe ff_get_video_buffer can be updated to accept an align argument 
which would be used instead of av_cpu_max_align() when not 0, so we 
don't lose the benefits of the frame pool it provides?

> +    if (ret < 0)
> +        av_frame_free(&frame);
> +
> +    return frame;
> +}
> +
>   static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>   {
>       Frei0rContext *s = inlink->dst->priv;
>       AVFilterLink *outlink = inlink->dst->outputs[0];
> -    AVFrame *out;
> +    AVFrame *out = getframe(outlink);
> +    if (!out)
> +        goto fail;
>   
> -    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> -    if (!out) {
> +    av_frame_copy_props(out, in);
> +
> +    if (in->linesize[0] != out->linesize[0]) {
> +        AVFrame *in2 = getframe(outlink);
> +        if (!in2)
> +            goto fail;
> +        av_frame_copy(in2, in);
>           av_frame_free(&in);
> -        return AVERROR(ENOMEM);
> +        in = in2;
>       }
> -    av_frame_copy_props(out, in);
>   
>       s->update(s->instance, in->pts * av_q2d(inlink->time_base) * 1000,
>                      (const uint32_t *)in->data[0],
> @@ -369,6 +392,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>       av_frame_free(&in);
>   
>       return ff_filter_frame(outlink, out);
> +fail:
> +    av_frame_free(&in);
> +    av_frame_free(&out);
> +    return AVERROR(ENOMEM);
>   }
>   
>   static int process_command(AVFilterContext *ctx, const char *cmd, const char *args,
> @@ -473,9 +500,6 @@ static int source_request_frame(AVFilterLink *outlink)
>       frame->sample_aspect_ratio = (AVRational) {1, 1};
>       frame->pts = s->pts++;
>   
> -    s->update(s->instance, av_rescale_q(frame->pts, s->time_base, (AVRational){1,1000}),
> -                   NULL, (uint32_t *)frame->data[0]);
> -
>       return ff_filter_frame(outlink, frame);
>   }
>
Michael Niedermayer April 11, 2022, 8:07 p.m. UTC | #3
On Mon, Apr 11, 2022 at 07:34:40PM +0200, Paul B Mahol wrote:
> On Mon, Apr 11, 2022 at 3:32 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > Fixes: issues with non trivial linesize
> >
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavfilter/vf_frei0r.c | 40 ++++++++++++++++++++++++++++++++--------
> >  1 file changed, 32 insertions(+), 8 deletions(-)
> >
> > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> > index 9cd0098e73..c9b698897f 100644
> > --- a/libavfilter/vf_frei0r.c
> > +++ b/libavfilter/vf_frei0r.c
> > @@ -349,18 +349,41 @@ static int query_formats(AVFilterContext *ctx)
> >      return ff_set_common_formats(ctx, formats);
> >  }
> >
> > +static AVFrame *getframe(AVFilterLink *link)
> > +{
> > +    int ret;
> > +    AVFrame *frame = av_frame_alloc();
> > +    if (!frame)
> > +        return NULL;
> > +
> > +    frame->width  = link->w;
> > +    frame->height = link->h;
> > +    frame->format = link->format;
> > +    ret = av_frame_get_buffer(frame, 16);
> > +    if (ret < 0)
> > +        av_frame_free(&frame);
> > +
> > +    return frame;
> > +}
> > +
> >  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> >  {
> >      Frei0rContext *s = inlink->dst->priv;
> >      AVFilterLink *outlink = inlink->dst->outputs[0];
> > -    AVFrame *out;
> > +    AVFrame *out = getframe(outlink);
> > +    if (!out)
> > +        goto fail;
> >
> > -    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> > -    if (!out) {
> > +    av_frame_copy_props(out, in);
> > +
> > +    if (in->linesize[0] != out->linesize[0]) {
> > +        AVFrame *in2 = getframe(outlink);
> > +        if (!in2)
> > +            goto fail;
> > +        av_frame_copy(in2, in);
> >          av_frame_free(&in);
> > -        return AVERROR(ENOMEM);
> > +        in = in2;
> >      }
> > -    av_frame_copy_props(out, in);
> >
> >      s->update(s->instance, in->pts * av_q2d(inlink->time_base) * 1000,
> >                     (const uint32_t *)in->data[0],
> > @@ -369,6 +392,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> > *in)
> >      av_frame_free(&in);
> >
> >      return ff_filter_frame(outlink, out);
> > +fail:
> > +    av_frame_free(&in);
> > +    av_frame_free(&out);
> > +    return AVERROR(ENOMEM);
> >  }
> >
> >  static int process_command(AVFilterContext *ctx, const char *cmd, const
> > char *args,
> > @@ -473,9 +500,6 @@ static int source_request_frame(AVFilterLink *outlink)
> >      frame->sample_aspect_ratio = (AVRational) {1, 1};
> >      frame->pts = s->pts++;
> >
> > -    s->update(s->instance, av_rescale_q(frame->pts, s->time_base,
> > (AVRational){1,1000}),
> > -                   NULL, (uint32_t *)frame->data[0]);
> >
> >
> Why this is removed?

i did miss that frei0r_src exists


> 
> That handle frei0r source filters.

I ll send a better patch

thx

[...]
Michael Niedermayer April 11, 2022, 8:46 p.m. UTC | #4
On Mon, Apr 11, 2022 at 03:01:57PM -0300, James Almer wrote:
> 
> 
> On 4/11/2022 10:31 AM, Michael Niedermayer wrote:
> > Fixes: issues with non trivial linesize
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavfilter/vf_frei0r.c | 40 ++++++++++++++++++++++++++++++++--------
> >   1 file changed, 32 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> > index 9cd0098e73..c9b698897f 100644
> > --- a/libavfilter/vf_frei0r.c
> > +++ b/libavfilter/vf_frei0r.c
> > @@ -349,18 +349,41 @@ static int query_formats(AVFilterContext *ctx)
> >       return ff_set_common_formats(ctx, formats);
> >   }
> > +static AVFrame *getframe(AVFilterLink *link)
> > +{
> > +    int ret;
> > +    AVFrame *frame = av_frame_alloc();
> > +    if (!frame)
> > +        return NULL;
> > +
> > +    frame->width  = link->w;
> > +    frame->height = link->h;
> > +    frame->format = link->format;
> > +    ret = av_frame_get_buffer(frame, 16);
> 
> Maybe ff_get_video_buffer can be updated to accept an align argument which
> would be used instead of av_cpu_max_align() when not 0, so we don't lose the
> benefits of the frame pool it provides?

We need a specific alignment and specific linesize.
ff_get_video_buffer() is forwarded to the next filter so a change to it
feels moderately messy. Each filter using it would have to deal with
specific linesize and alignment requirements. Thats for one odd filter
What can be done is to work with ff_default_get_video_buffer() maybe
and never use the next filters one.
Ill send a patch doing that if it pases tests

thx

[...]
diff mbox series

Patch

diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
index 9cd0098e73..c9b698897f 100644
--- a/libavfilter/vf_frei0r.c
+++ b/libavfilter/vf_frei0r.c
@@ -349,18 +349,41 @@  static int query_formats(AVFilterContext *ctx)
     return ff_set_common_formats(ctx, formats);
 }
 
+static AVFrame *getframe(AVFilterLink *link)
+{
+    int ret;
+    AVFrame *frame = av_frame_alloc();
+    if (!frame)
+        return NULL;
+
+    frame->width  = link->w;
+    frame->height = link->h;
+    frame->format = link->format;
+    ret = av_frame_get_buffer(frame, 16);
+    if (ret < 0)
+        av_frame_free(&frame);
+
+    return frame;
+}
+
 static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 {
     Frei0rContext *s = inlink->dst->priv;
     AVFilterLink *outlink = inlink->dst->outputs[0];
-    AVFrame *out;
+    AVFrame *out = getframe(outlink);
+    if (!out)
+        goto fail;
 
-    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
-    if (!out) {
+    av_frame_copy_props(out, in);
+
+    if (in->linesize[0] != out->linesize[0]) {
+        AVFrame *in2 = getframe(outlink);
+        if (!in2)
+            goto fail;
+        av_frame_copy(in2, in);
         av_frame_free(&in);
-        return AVERROR(ENOMEM);
+        in = in2;
     }
-    av_frame_copy_props(out, in);
 
     s->update(s->instance, in->pts * av_q2d(inlink->time_base) * 1000,
                    (const uint32_t *)in->data[0],
@@ -369,6 +392,10 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
     av_frame_free(&in);
 
     return ff_filter_frame(outlink, out);
+fail:
+    av_frame_free(&in);
+    av_frame_free(&out);
+    return AVERROR(ENOMEM);
 }
 
 static int process_command(AVFilterContext *ctx, const char *cmd, const char *args,
@@ -473,9 +500,6 @@  static int source_request_frame(AVFilterLink *outlink)
     frame->sample_aspect_ratio = (AVRational) {1, 1};
     frame->pts = s->pts++;
 
-    s->update(s->instance, av_rescale_q(frame->pts, s->time_base, (AVRational){1,1000}),
-                   NULL, (uint32_t *)frame->data[0]);
-
     return ff_filter_frame(outlink, frame);
 }