[FFmpeg-devel] [PATCH v3 2/3] libavfilter/vf_cover_rect: support for cover image with more pixel format and different width and height

[Lance Wang]

On Wed, Jun 12, 2019 at 4:51 AM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Tue, Jun 11, 2019 at 01:38:43PM +0800, lance.lmwang at gmail.com wrote:
> > From: Limin Wang <lance.lmwang at gmail.com>
> >
> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > ---
> >  libavfilter/vf_cover_rect.c | 49 +++++++++++++++++++++++++++----------
> >  1 file changed, 36 insertions(+), 13 deletions(-)
> >
> > diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> > index 898debf09d..22389b79d2 100644
> > --- a/libavfilter/vf_cover_rect.c
> > +++ b/libavfilter/vf_cover_rect.c
> > @@ -28,6 +28,7 @@
> >  #include "internal.h"
> >
> >  #include "lavfutils.h"
> > +#include "lswsutils.h"
> >
> >  enum mode {
> >      MODE_COVER,
> > @@ -40,6 +41,7 @@ typedef struct CoverContext {
> >      int mode;
> >      char *cover_filename;
> >      AVFrame *cover_frame;
> > +    AVFrame *match_frame;
> >      int width, height;
> >  } CoverContext;
> >
> > @@ -71,21 +73,21 @@ 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];
> >          }
> >      }
> >  }
> > @@ -138,7 +140,10 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >      CoverContext *cover = ctx->priv;
> >      AVDictionaryEntry *ex, *ey, *ew, *eh;
> >      int x = -1, y = -1, w = -1, h = -1;
> > +    enum AVPixelFormat in_format;
> >      char *xendptr = NULL, *yendptr = NULL, *wendptr = NULL, *hendptr =
> NULL;
> > +    AVFrame *cover_frame = NULL;
> > +    int ret;
> >
> >      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);
> > @@ -167,13 +172,30 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >      }
> >      w = FFMIN(w, in->width  - x);
> >      h = FFMIN(h, in->height - y);
> > +    in_format = in->format;
> >
> >      if (w > in->width || h > in->height || w <= 0 || h <= 0)
> >          return AVERROR(EINVAL);
> >
> >      if (cover->cover_frame) {
> > -        if (w != cover->cover_frame->width || h !=
> cover->cover_frame->height)
> > -            return AVERROR(EINVAL);
> > +        if (w != cover->cover_frame->width || h !=
> cover->cover_frame->height ||
> > +                in_format != cover->cover_frame->format) {
> > +            if (!cover->match_frame && !(cover->match_frame =
> av_frame_alloc()))
> > +                return AVERROR(EINVAL);
> > +
> > +            if ((ret = ff_scale_image(cover->match_frame->data,
> cover->match_frame->linesize,
> > +                            w, h, in_format, cover->cover_frame->data,
> cover->cover_frame->linesize,
> > +                            cover->cover_frame->width,
> cover->cover_frame->height,
> > +                            cover->cover_frame->format, ctx)) < 0)
> > +                return AVERROR(EINVAL);
>
> This looks like the same static cover image would be converted again for
> each input frame
> which if iam not misreading this would be doing the exact same operation
> over and over
> again, thats same wastefull
>
>
I have fixed the issue in the updated patch, please help to review again.
https://patchwork.ffmpeg.org/patch/13510/


> [....]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".