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

[Lance Wang]

On Fri, Jun 14, 2019 at 5:31 PM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Wed, Jun 12, 2019 at 06:50:18PM +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>
> > ---
> >  doc/filters.texi            |  6 ++--
> >  libavfilter/vf_cover_rect.c | 56 +++++++++++++++++++++++++++----------
> >  2 files changed, 44 insertions(+), 18 deletions(-)
> >
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index ec1c7c7591..4594a61c13 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -10166,7 +10166,7 @@ Specifies the rectangle in which to search.
> >
> >  @itemize
> >  @item
> > -Generate a representative palette of a given video using
> @command{ffmpeg}:
> > +Cover a rectangular object by the supplied image of a given video using
> @command{ffmpeg}:
> >  @example
> >  ffmpeg -i file.ts -vf
> find_rect=newref.pgm,cover_rect=cover.jpg:mode=cover new.mkv
> >  @end example
> > @@ -10180,7 +10180,7 @@ It accepts the following options:
> >
> >  @table @option
> >  @item cover
> > -Filepath of the optional cover image, needs to be in yuv420.
> > +Filepath of the optional cover image.
> >
> >  @item mode
> >  Set covering mode.
> > @@ -10200,7 +10200,7 @@ Default value is @var{blur}.
> >
> >  @itemize
> >  @item
> > -Generate a representative palette of a given video using
> @command{ffmpeg}:
> > +Cover a rectangular object by the supplied image of a given video using
> @command{ffmpeg}:
> >  @example
> >  ffmpeg -i file.ts -vf
> find_rect=newref.pgm,cover_rect=cover.jpg:mode=cover new.mkv
> >  @end example
> > diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> > index 898debf09d..36c650dd21 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];
> >          }
> >      }
> >  }
>
> This hunk is independant and can be done in a seperate patch before
>
>
OK, I'll split the patch.


>
> > @@ -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,14 +172,34 @@ 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) {
>
> This segfaults
>

cover->cover_frame is checked in the init function, if it's null, it'll
failed already, so I remove the checking and haven't got any segments issue.
anyway, I'll add it for the updated patch.


>
>
> > +        if (!cover->match_frame || (w != cover->match_frame->width || h
> != cover->match_frame->height
> > +                    || in_format != cover->match_frame->format)) {
> > +            if (cover->match_frame)
> > +                av_freep(&cover->match_frame->data[0]);
> > +            else if (!(cover->match_frame = av_frame_alloc()))
> > +                return AVERROR(ENOMEM);
> > +
>
> > +            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(ENOMEM);
>
> This sort of reimplements the scale filter and it
> doesnt consider some parameters like AVCOL_RANGE*
>

the ff_scale_image is implemented and used by other alike place,  I try to
reuse the function anyway.
If we need other parameters for scale, we may improve the function later,
now it's OK for my testing as
the cover image is logo mostly.




>
> Thanks
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Awnsering whenever a program halts or runs forever is
> On a turing machine, in general impossible (turings halting problem).
> On any real computer, always possible as a real computer has a finite
> number
> of states N, and will either halt in less than N cycles or never halt.
> _______________________________________________
> 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".