diff mbox

[FFmpeg-devel,v1,1/2] avfilter/vf_scale: split the scale_frame function from filter_frame for activate function support

Message ID 20190727121817.22390-1-lance.lmwang@gmail.com
State Superseded
Headers show

Commit Message

Lance Wang July 27, 2019, 12:18 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/vf_scale.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

Comments

Lance Wang Sept. 2, 2019, 1:52 p.m. UTC | #1 
ping, although my frame thread code can't pass fate testing and can't
submit for review. The change is independent for that.

Thanks,
Limin
On Sat, Jul 27, 2019 at 08:18:16PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_scale.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 7aebf56..efb480d 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -400,7 +400,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s
>                           out,out_stride);
>  }
>  
> -static int filter_frame(AVFilterLink *link, AVFrame *in)
> +static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out, int *got_frame)
>  {
>      ScaleContext *scale = link->dst->priv;
>      AVFilterLink *outlink = link->dst->outputs[0];
> @@ -409,6 +409,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>      char buf[32];
>      int in_range;
>  
> +    *got_frame = 0;
>      if (in->colorspace == AVCOL_SPC_YCGCO)
>          av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n");
>  
> @@ -437,8 +438,11 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>              return ret;
>      }
>  
> -    if (!scale->sws)
> -        return ff_filter_frame(outlink, in);
> +    if (!scale->sws) {
> +        *got_frame = 1;
> +        *frame_out = in;
> +        return 0;
> +    }
>  
>      scale->hsub = desc->log2_chroma_w;
>      scale->vsub = desc->log2_chroma_h;
> @@ -448,6 +452,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>          av_frame_free(&in);
>          return AVERROR(ENOMEM);
>      }
> +    *frame_out = out;
>  
>      av_frame_copy_props(out, in);
>      out->width  = outlink->w;
> @@ -521,7 +526,23 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>      }
>  
>      av_frame_free(&in);
> -    return ff_filter_frame(outlink, out);
> +    *got_frame = 1;
> +    return 0;
> +}
> +
> +static int filter_frame(AVFilterLink *link, AVFrame *in)
> +{
> +    AVFilterContext *ctx = link->dst;
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    int got_frame = 0;
> +    AVFrame *out;
> +    int ret;
> +
> +    ret = scale_frame(link, in, &out, &got_frame);
> +    if (got_frame)
> +        return ff_filter_frame(outlink, out);
> +
> +    return ret;
>  }
>  
>  static int filter_frame_ref(AVFilterLink *link, AVFrame *in)
> -- 
> 2.6.4
>
Michael Niedermayer Sept. 3, 2019, 10:07 p.m. UTC | #2 
On Sat, Jul 27, 2019 at 08:18:16PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_scale.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)

LGTM

thx

[...]
Lance Wang Sept. 6, 2019, 2:44 p.m. UTC | #3 
On Wed, Sep 04, 2019 at 12:07:32AM +0200, Michael Niedermayer wrote:
> On Sat, Jul 27, 2019 at 08:18:16PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavfilter/vf_scale.c | 29 +++++++++++++++++++++++++----
> >  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> LGTM
> 
ping, please help to merge it.


> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> If you fake or manipulate statistics in a paper in physics you will never
> get a job again.
> If you fake or manipulate statistics in a paper in medicin you will get
> a job for life at the pharma industry.



> _______________________________________________
> 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".
Michael Niedermayer Sept. 6, 2019, 6:08 p.m. UTC | #4 
On Sat, Jul 27, 2019 at 08:18:16PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_scale.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 7aebf56..efb480d 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -400,7 +400,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s
>                           out,out_stride);
>  }
>  
> -static int filter_frame(AVFilterLink *link, AVFrame *in)
> +static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out, int *got_frame)
>  {
>      ScaleContext *scale = link->dst->priv;
>      AVFilterLink *outlink = link->dst->outputs[0];
> @@ -409,6 +409,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>      char buf[32];
>      int in_range;
>  
> +    *got_frame = 0;
>      if (in->colorspace == AVCOL_SPC_YCGCO)
>          av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n");
>  
> @@ -437,8 +438,11 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>              return ret;
>      }
>  
> -    if (!scale->sws)
> -        return ff_filter_frame(outlink, in);
> +    if (!scale->sws) {
> +        *got_frame = 1;
> +        *frame_out = in;
> +        return 0;
> +    }
>  
>      scale->hsub = desc->log2_chroma_w;
>      scale->vsub = desc->log2_chroma_h;
> @@ -448,6 +452,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>          av_frame_free(&in);
>          return AVERROR(ENOMEM);
>      }
> +    *frame_out = out;
>  
>      av_frame_copy_props(out, in);
>      out->width  = outlink->w;
> @@ -521,7 +526,23 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>      }
>  
>      av_frame_free(&in);
> -    return ff_filter_frame(outlink, out);
> +    *got_frame = 1;
> +    return 0;
> +}
> +
> +static int filter_frame(AVFilterLink *link, AVFrame *in)
> +{
> +    AVFilterContext *ctx = link->dst;
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    int got_frame = 0;
> +    AVFrame *out;
> +    int ret;
> +
> +    ret = scale_frame(link, in, &out, &got_frame);
> +    if (got_frame)
> +        return ff_filter_frame(outlink, out);

can this be simplified by using out != NULL instead of got_frame ?

thx

[...]
Lance Wang Sept. 6, 2019, 10:25 p.m. UTC | #5 
On Fri, Sep 06, 2019 at 08:08:48PM +0200, Michael Niedermayer wrote:
> On Sat, Jul 27, 2019 at 08:18:16PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavfilter/vf_scale.c | 29 +++++++++++++++++++++++++----
> >  1 file changed, 25 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> > index 7aebf56..efb480d 100644
> > --- a/libavfilter/vf_scale.c
> > +++ b/libavfilter/vf_scale.c
> > @@ -400,7 +400,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s
> >                           out,out_stride);
> >  }
> >  
> > -static int filter_frame(AVFilterLink *link, AVFrame *in)
> > +static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out, int *got_frame)
> >  {
> >      ScaleContext *scale = link->dst->priv;
> >      AVFilterLink *outlink = link->dst->outputs[0];
> > @@ -409,6 +409,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >      char buf[32];
> >      int in_range;
> >  
> > +    *got_frame = 0;
> >      if (in->colorspace == AVCOL_SPC_YCGCO)
> >          av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n");
> >  
> > @@ -437,8 +438,11 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >              return ret;
> >      }
> >  
> > -    if (!scale->sws)
> > -        return ff_filter_frame(outlink, in);
> > +    if (!scale->sws) {
> > +        *got_frame = 1;
> > +        *frame_out = in;
> > +        return 0;
> > +    }
> >  
> >      scale->hsub = desc->log2_chroma_w;
> >      scale->vsub = desc->log2_chroma_h;
> > @@ -448,6 +452,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >          av_frame_free(&in);
> >          return AVERROR(ENOMEM);
> >      }
> > +    *frame_out = out;
> >  
> >      av_frame_copy_props(out, in);
> >      out->width  = outlink->w;
> > @@ -521,7 +526,23 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >      }
> >  
> >      av_frame_free(&in);
> > -    return ff_filter_frame(outlink, out);
> > +    *got_frame = 1;
> > +    return 0;
> > +}
> > +
> > +static int filter_frame(AVFilterLink *link, AVFrame *in)
> > +{
> > +    AVFilterContext *ctx = link->dst;
> > +    AVFilterLink *outlink = ctx->outputs[0];
> > +    int got_frame = 0;
> > +    AVFrame *out;
> > +    int ret;
> > +
> > +    ret = scale_frame(link, in, &out, &got_frame);
> > +    if (got_frame)
> > +        return ff_filter_frame(outlink, out);
> 
> can this be simplified by using out != NULL instead of got_frame ?
> 
Yes, I'm just following the common video and audio decode style, they're
define got_frame_ptr always, I think it's more readiable and make code
less bugy. If you still prefer to remove got_frame, I can update the patch.


> thx
> 
> [...]
> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates



> _______________________________________________
> 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".
Lance Wang Sept. 7, 2019, 3:13 p.m. UTC | #6 
On Fri, Sep 06, 2019 at 08:08:48PM +0200, Michael Niedermayer wrote:
> On Sat, Jul 27, 2019 at 08:18:16PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavfilter/vf_scale.c | 29 +++++++++++++++++++++++++----
> >  1 file changed, 25 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> > index 7aebf56..efb480d 100644
> > --- a/libavfilter/vf_scale.c
> > +++ b/libavfilter/vf_scale.c
> > @@ -400,7 +400,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s
> >                           out,out_stride);
> >  }
> >  
> > -static int filter_frame(AVFilterLink *link, AVFrame *in)
> > +static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out, int *got_frame)
> >  {
> >      ScaleContext *scale = link->dst->priv;
> >      AVFilterLink *outlink = link->dst->outputs[0];
> > @@ -409,6 +409,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >      char buf[32];
> >      int in_range;
> >  
> > +    *got_frame = 0;
> >      if (in->colorspace == AVCOL_SPC_YCGCO)
> >          av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n");
> >  
> > @@ -437,8 +438,11 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >              return ret;
> >      }
> >  
> > -    if (!scale->sws)
> > -        return ff_filter_frame(outlink, in);
> > +    if (!scale->sws) {
> > +        *got_frame = 1;
> > +        *frame_out = in;
> > +        return 0;
> > +    }
> >  
> >      scale->hsub = desc->log2_chroma_w;
> >      scale->vsub = desc->log2_chroma_h;
> > @@ -448,6 +452,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >          av_frame_free(&in);
> >          return AVERROR(ENOMEM);
> >      }
> > +    *frame_out = out;
> >  
> >      av_frame_copy_props(out, in);
> >      out->width  = outlink->w;
> > @@ -521,7 +526,23 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >      }
> >  
> >      av_frame_free(&in);
> > -    return ff_filter_frame(outlink, out);
> > +    *got_frame = 1;
> > +    return 0;
> > +}
> > +
> > +static int filter_frame(AVFilterLink *link, AVFrame *in)
> > +{
> > +    AVFilterContext *ctx = link->dst;
> > +    AVFilterLink *outlink = ctx->outputs[0];
> > +    int got_frame = 0;
> > +    AVFrame *out;
> > +    int ret;
> > +
> > +    ret = scale_frame(link, in, &out, &got_frame);
> > +    if (got_frame)
> > +        return ff_filter_frame(outlink, out);
> 
> can this be simplified by using out != NULL instead of got_frame ?

Michael I have updated the patch to simplified the code. fate testing is 
passed, please review it.

> 
> thx
> 
> [...]
> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates



> _______________________________________________
> 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

Patch

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 7aebf56..efb480d 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -400,7 +400,7 @@  static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s
                          out,out_stride);
 }
 
-static int filter_frame(AVFilterLink *link, AVFrame *in)
+static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out, int *got_frame)
 {
     ScaleContext *scale = link->dst->priv;
     AVFilterLink *outlink = link->dst->outputs[0];
@@ -409,6 +409,7 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
     char buf[32];
     int in_range;
 
+    *got_frame = 0;
     if (in->colorspace == AVCOL_SPC_YCGCO)
         av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n");
 
@@ -437,8 +438,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
             return ret;
     }
 
-    if (!scale->sws)
-        return ff_filter_frame(outlink, in);
+    if (!scale->sws) {
+        *got_frame = 1;
+        *frame_out = in;
+        return 0;
+    }
 
     scale->hsub = desc->log2_chroma_w;
     scale->vsub = desc->log2_chroma_h;
@@ -448,6 +452,7 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
         av_frame_free(&in);
         return AVERROR(ENOMEM);
     }
+    *frame_out = out;
 
     av_frame_copy_props(out, in);
     out->width  = outlink->w;
@@ -521,7 +526,23 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
     }
 
     av_frame_free(&in);
-    return ff_filter_frame(outlink, out);
+    *got_frame = 1;
+    return 0;
+}
+
+static int filter_frame(AVFilterLink *link, AVFrame *in)
+{
+    AVFilterContext *ctx = link->dst;
+    AVFilterLink *outlink = ctx->outputs[0];
+    int got_frame = 0;
+    AVFrame *out;
+    int ret;
+
+    ret = scale_frame(link, in, &out, &got_frame);
+    if (got_frame)
+        return ff_filter_frame(outlink, out);
+
+    return ret;
 }
 
 static int filter_frame_ref(AVFilterLink *link, AVFrame *in)