On Sun, Jun 16, 2019 at 6:20 AM Michael Niedermayer <mich...@niedermayer.cc>
wrote:
> On Fri, Jun 14, 2019 at 11:52:47PM +0800, Lance Wang wrote:
> > On Fri, Jun 14, 2019 at 5:31 PM Michael Niedermayer
> <mich...@niedermayer.cc>
> > wrote:
> >
> > > On Wed, Jun 12, 2019 at 06:50:18PM +0800, lance.lmw...@gmail.com
> wrote:
> > > > From: Limin Wang <lance.lmw...@gmail.com>
> > > >
> > > > Signed-off-by: Limin Wang <lance.lmw...@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.
>
> there are 2 modes, blur doesnt need an image
>
>
I have updated and split the patch yesterday, it's OK to run for all mode
by my testing. Please check them.
>
> >
> >
> > >
> > >
> > > > + 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.
>
> I think one could argue that the scale filter should be used for converting
> this way the cover image would become one externally vissible input. (which
> then subsequently also could change over time and not just be a static
> image ...)
>
>
For now, the code is simple and prefer to use it. How about to improve all
other function which use ff_scale_image in future.
> thx
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Elect your leaders based on what they did after the last election, not
> based on what they say before an election.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
