diff mbox

[FFmpeg-devel,v5,2/3] ibavfilter/vf_cover_rect.c: change the cover_rect function for support cover frame can be changed

Message ID 1560530906-60749-2-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show

Commit Message

Lance Wang June 14, 2019, 4:48 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/vf_cover_rect.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Michael Niedermayer June 16, 2019, 7:23 a.m. UTC | #1 
On Sat, Jun 15, 2019 at 12:48:25AM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_cover_rect.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> index 898debf..b66c40b 100644
> --- a/libavfilter/vf_cover_rect.c
> +++ b/libavfilter/vf_cover_rect.c
> @@ -71,24 +71,25 @@ static int config_input(AVFilterLink *inlink)
>      return 0;
>  }
>  
> -static void cover_rect(CoverContext *cover, AVFrame *in, int offx, int offy)
> +static void cover_rect(AVFrame *cover_frame, AVFrame *in, int offx, int offy)
>  {
>      int x, y, p;
>  
>      for (p = 0; p < 3; p++) {
>          uint8_t *data = in->data[p] + (offx>>!!p) + (offy>>!!p) * in->linesize[p];
> -        const uint8_t *src = cover->cover_frame->data[p];
> -        int w = AV_CEIL_RSHIFT(cover->cover_frame->width , !!p);
> -        int h = AV_CEIL_RSHIFT(cover->cover_frame->height, !!p);
> +        const uint8_t *src = cover_frame->data[p];
> +        int w = AV_CEIL_RSHIFT(cover_frame->width , !!p);
> +        int h = AV_CEIL_RSHIFT(cover_frame->height, !!p);
>          for (y = 0; y < h; y++) {
>              for (x = 0; x < w; x++) {
>                  data[x] = src[x];
>              }
>              data += in->linesize[p];
> -            src += cover->cover_frame->linesize[p];
> +            src += cover_frame->linesize[p];
>          }
>      }

>  }
> +
>  static void blur(CoverContext *cover, AVFrame *in, int offx, int offy)
>  {
>      int x, y, p;
> @@ -139,6 +140,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>      AVDictionaryEntry *ex, *ey, *ew, *eh;
>      int x = -1, y = -1, w = -1, h = -1;
>      char *xendptr = NULL, *yendptr = NULL, *wendptr = NULL, *hendptr = NULL;
> +    AVFrame *cover_frame = NULL;
>  
>      ex = av_dict_get(in->metadata, "lavfi.rect.x", NULL, AV_DICT_MATCH_CASE);
>      ey = av_dict_get(in->metadata, "lavfi.rect.y", NULL, AV_DICT_MATCH_CASE);
> @@ -174,6 +176,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>      if (cover->cover_frame) {
>          if (w != cover->cover_frame->width || h != cover->cover_frame->height)
>              return AVERROR(EINVAL);
> +        cover_frame = cover->cover_frame;
>      }
>  
>      cover->width  = w;
> @@ -186,8 +189,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  
>      if (cover->mode == MODE_BLUR) {
>          blur (cover, in, x, y);

> -    } else {
> -        cover_rect(cover, in, x, y);
> +    } else if (cover_frame) {
> +        cover_rect(cover_frame, in, x, y);

replacing a field from the context by a local variable cannot make a field
null that was not before


[...]
Lance Wang June 16, 2019, 9:35 a.m. UTC | #2 
On Sun, Jun 16, 2019 at 3:24 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Sat, Jun 15, 2019 at 12:48:25AM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> >
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavfilter/vf_cover_rect.c | 17 ++++++++++-------
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> > index 898debf..b66c40b 100644
> > --- a/libavfilter/vf_cover_rect.c
> > +++ b/libavfilter/vf_cover_rect.c
> > @@ -71,24 +71,25 @@ static int config_input(AVFilterLink *inlink)
> >      return 0;
> >  }
> >
> > -static void cover_rect(CoverContext *cover, AVFrame *in, int offx, int
> offy)
> > +static void cover_rect(AVFrame *cover_frame, AVFrame *in, int offx, int
> offy)
> >  {
> >      int x, y, p;
> >
> >      for (p = 0; p < 3; p++) {
> >          uint8_t *data = in->data[p] + (offx>>!!p) + (offy>>!!p) *
> in->linesize[p];
> > -        const uint8_t *src = cover->cover_frame->data[p];
> > -        int w = AV_CEIL_RSHIFT(cover->cover_frame->width , !!p);
> > -        int h = AV_CEIL_RSHIFT(cover->cover_frame->height, !!p);
> > +        const uint8_t *src = cover_frame->data[p];
> > +        int w = AV_CEIL_RSHIFT(cover_frame->width , !!p);
> > +        int h = AV_CEIL_RSHIFT(cover_frame->height, !!p);
> >          for (y = 0; y < h; y++) {
> >              for (x = 0; x < w; x++) {
> >                  data[x] = src[x];
> >              }
> >              data += in->linesize[p];
> > -            src += cover->cover_frame->linesize[p];
> > +            src += cover_frame->linesize[p];
> >          }
> >      }
>
> >  }
> > +
> >  static void blur(CoverContext *cover, AVFrame *in, int offx, int offy)
> >  {
> >      int x, y, p;
> > @@ -139,6 +140,7 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >      AVDictionaryEntry *ex, *ey, *ew, *eh;
> >      int x = -1, y = -1, w = -1, h = -1;
> >      char *xendptr = NULL, *yendptr = NULL, *wendptr = NULL, *hendptr =
> NULL;
> > +    AVFrame *cover_frame = NULL;
> >
> >      ex = av_dict_get(in->metadata, "lavfi.rect.x", NULL,
> AV_DICT_MATCH_CASE);
> >      ey = av_dict_get(in->metadata, "lavfi.rect.y", NULL,
> AV_DICT_MATCH_CASE);
> > @@ -174,6 +176,7 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >      if (cover->cover_frame) {
> >          if (w != cover->cover_frame->width || h !=
> cover->cover_frame->height)
> >              return AVERROR(EINVAL);
> > +        cover_frame = cover->cover_frame;
> >      }
> >
> >      cover->width  = w;
> > @@ -186,8 +189,8 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >
> >      if (cover->mode == MODE_BLUR) {
> >          blur (cover, in, x, y);
>
> > -    } else {
> > -        cover_rect(cover, in, x, y);
> > +    } else if (cover_frame) {
> > +        cover_rect(cover_frame, in, x, y);
>
> replacing a field from the context by a local variable cannot make a field
> null that was not before
>

Yes,  no need to check here.  I'll update if the ff_scale_image function
issue can be fixed.



>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Its not that you shouldnt use gotos but rather that you should write
> readable code and code with gotos often but not always is less readable
> _______________________________________________
> 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_cover_rect.c b/libavfilter/vf_cover_rect.c
index 898debf..b66c40b 100644
--- a/libavfilter/vf_cover_rect.c
+++ b/libavfilter/vf_cover_rect.c
@@ -71,24 +71,25 @@  static int config_input(AVFilterLink *inlink)
     return 0;
 }
 
-static void cover_rect(CoverContext *cover, AVFrame *in, int offx, int offy)
+static void cover_rect(AVFrame *cover_frame, AVFrame *in, int offx, int offy)
 {
     int x, y, p;
 
     for (p = 0; p < 3; p++) {
         uint8_t *data = in->data[p] + (offx>>!!p) + (offy>>!!p) * in->linesize[p];
-        const uint8_t *src = cover->cover_frame->data[p];
-        int w = AV_CEIL_RSHIFT(cover->cover_frame->width , !!p);
-        int h = AV_CEIL_RSHIFT(cover->cover_frame->height, !!p);
+        const uint8_t *src = cover_frame->data[p];
+        int w = AV_CEIL_RSHIFT(cover_frame->width , !!p);
+        int h = AV_CEIL_RSHIFT(cover_frame->height, !!p);
         for (y = 0; y < h; y++) {
             for (x = 0; x < w; x++) {
                 data[x] = src[x];
             }
             data += in->linesize[p];
-            src += cover->cover_frame->linesize[p];
+            src += cover_frame->linesize[p];
         }
     }
 }
+
 static void blur(CoverContext *cover, AVFrame *in, int offx, int offy)
 {
     int x, y, p;
@@ -139,6 +140,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
     AVDictionaryEntry *ex, *ey, *ew, *eh;
     int x = -1, y = -1, w = -1, h = -1;
     char *xendptr = NULL, *yendptr = NULL, *wendptr = NULL, *hendptr = NULL;
+    AVFrame *cover_frame = NULL;
 
     ex = av_dict_get(in->metadata, "lavfi.rect.x", NULL, AV_DICT_MATCH_CASE);
     ey = av_dict_get(in->metadata, "lavfi.rect.y", NULL, AV_DICT_MATCH_CASE);
@@ -174,6 +176,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
     if (cover->cover_frame) {
         if (w != cover->cover_frame->width || h != cover->cover_frame->height)
             return AVERROR(EINVAL);
+        cover_frame = cover->cover_frame;
     }
 
     cover->width  = w;
@@ -186,8 +189,8 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 
     if (cover->mode == MODE_BLUR) {
         blur (cover, in, x, y);
-    } else {
-        cover_rect(cover, in, x, y);
+    } else if (cover_frame) {
+        cover_rect(cover_frame, in, x, y);
     }
     return ff_filter_frame(ctx->outputs[0], in);
 }