| Message ID | 20190612105018.43545-1-lance.lmwang@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Jun 12, 2019 at 06:50:18PM +0800, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@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 > @@ -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 > + 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* Thanks [...]
On Fri, Jun 14, 2019 at 5:31 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Jun 12, 2019 at 06:50:18PM +0800, lance.lmwang@gmail.com wrote: > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Fri, Jun 14, 2019 at 11:52:47PM +0800, Lance Wang wrote: > On Fri, Jun 14, 2019 at 5:31 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Wed, Jun 12, 2019 at 06:50:18PM +0800, lance.lmwang@gmail.com wrote: > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > Signed-off-by: Limin Wang <lance.lmwang@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 > > > > > > > > > + 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 ...) thx [...]
On Sun, Jun 16, 2019 at 6:20 AM Michael Niedermayer <michael@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 > <michael@niedermayer.cc> > > wrote: > > > > > On Wed, Jun 12, 2019 at 06:50:18PM +0800, lance.lmwang@gmail.com > wrote: > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > > > Signed-off-by: Limin Wang <lance.lmwang@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-request@ffmpeg.org with subject "unsubscribe".
On Sun, Jun 16, 2019 at 07:11:27AM +0800, Lance Wang wrote: > On Sun, Jun 16, 2019 at 6:20 AM Michael Niedermayer <michael@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 > > <michael@niedermayer.cc> > > > wrote: [...] > > > > > > > > > > > > > > > > > > > > > + 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. if you add no new ff_scale_image(), sure theres no need to clean it up but if you add more bad code, no thats not ok IMO. Thanks [...]
On Sun, Jun 16, 2019 at 3:26 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sun, Jun 16, 2019 at 07:11:27AM +0800, Lance Wang wrote: > > On Sun, Jun 16, 2019 at 6:20 AM Michael Niedermayer > <michael@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 > > > <michael@niedermayer.cc> > > > > wrote: > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > + 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. > > if you add no new ff_scale_image(), sure theres no need to clean it up > but if you add more bad code, no thats not ok IMO. > > OK, if not use ff_scale_image function, what's better recommended to reuse the vf_scale complete function simply? > Thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Asymptotically faster algorithms should always be preferred if you have > asymptotical amounts of data > _______________________________________________ > 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 --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]; } } } @@ -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) { + 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); + + cover->match_frame->width = w; + cover->match_frame->height = h; + cover->match_frame->format = in_format; + } + + cover_frame = cover->match_frame; + } else + cover_frame = cover->cover_frame; cover->width = w; cover->height = h; @@ -187,8 +212,10 @@ 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); + cover_rect(cover_frame, in, x, y); + } + return ff_filter_frame(ctx->outputs[0], in); } @@ -199,6 +226,10 @@ static av_cold void uninit(AVFilterContext *ctx) if (cover->cover_frame) av_freep(&cover->cover_frame->data[0]); av_frame_free(&cover->cover_frame); + + if (cover->match_frame) + av_freep(&cover->match_frame->data[0]); + av_frame_free(&cover->match_frame); } static av_cold int init(AVFilterContext *ctx) @@ -220,11 +251,6 @@ static av_cold int init(AVFilterContext *ctx) &cover->cover_frame->width, &cover->cover_frame->height, &cover->cover_frame->format, cover->cover_filename, ctx)) < 0) return ret; - - if (cover->cover_frame->format != AV_PIX_FMT_YUV420P && cover->cover_frame->format != AV_PIX_FMT_YUVJ420P) { - av_log(ctx, AV_LOG_ERROR, "cover image is not a YUV420 image\n"); - return AVERROR(EINVAL); - } } return 0;