| Message ID | 20190612105731.43622-1-lance.lmwang@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Jun 12, 2019 at 06:57:29PM +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 | 2 +- > libavfilter/vf_find_rect.c | 39 +++++++++++++++++++++++++++----------- > 2 files changed, 29 insertions(+), 12 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index ec1c7c7591..90c57430a6 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -10150,7 +10150,7 @@ It accepts the following options: > > @table @option > @item object > -Filepath of the object image, needs to be in gray8. > +Filepath of the object image. > > @item threshold > Detection threshold, default is 0.5. > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > index d7e6579af7..ee6c3f4b45 100644 > --- a/libavfilter/vf_find_rect.c > +++ b/libavfilter/vf_find_rect.c > @@ -28,6 +28,7 @@ > #include "internal.h" > > #include "lavfutils.h" > +#include "lswsutils.h" > > #define MAX_MIPMAPS 5 > > @@ -244,6 +245,9 @@ static av_cold int init(AVFilterContext *ctx) > { > FOCContext *foc = ctx->priv; > int ret, i; > + uint8_t *tmp_data[4] = { NULL }; > + int tmp_linesize[4], width, height; > + enum AVPixelFormat pix_fmt; > > if (!foc->obj_filename) { > av_log(ctx, AV_LOG_ERROR, "object filename not set\n"); > @@ -254,24 +258,37 @@ static av_cold int init(AVFilterContext *ctx) > if (!foc->obj_frame) > return AVERROR(ENOMEM); > > - if ((ret = ff_load_image(foc->obj_frame->data, foc->obj_frame->linesize, > - &foc->obj_frame->width, &foc->obj_frame->height, > - &foc->obj_frame->format, foc->obj_filename, ctx)) < 0) > - return ret; > - > - if (foc->obj_frame->format != AV_PIX_FMT_GRAY8) { > - av_log(ctx, AV_LOG_ERROR, "object image is not a grayscale image\n"); > - return AVERROR(EINVAL); > - } > + if ((ret = ff_load_image(tmp_data, tmp_linesize, > + &width, &height, > + &pix_fmt, foc->obj_filename, ctx)) < 0) > + goto error; > + > + /* convert object image to gray8 format with same width and height */ > + foc->obj_frame->format = AV_PIX_FMT_GRAY8; > + foc->obj_frame->width = width; > + foc->obj_frame->height = height; > + if ((ret = ff_scale_image(foc->obj_frame->data, foc->obj_frame->linesize, > + foc->obj_frame->width, foc->obj_frame->height, foc->obj_frame->format, > + tmp_data, tmp_linesize, width, height, pix_fmt, ctx)) < 0) > + goto error; > + av_freep(&tmp_data[0]); Iam not sure i understand what this patch is intending to do Before a grayscale image was needed, after the patch anything is accepted but only grayscale is used. This is quite unexpected for a user who might expect color to be used if color is accpeted. Generally, doing something that is unexpected by the user is not good [...]
On Sat, Jun 15, 2019 at 3:16 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Jun 12, 2019 at 06:57:29PM +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 | 2 +- > > libavfilter/vf_find_rect.c | 39 +++++++++++++++++++++++++++----------- > > 2 files changed, 29 insertions(+), 12 deletions(-) > > > > diff --git a/doc/filters.texi b/doc/filters.texi > > index ec1c7c7591..90c57430a6 100644 > > --- a/doc/filters.texi > > +++ b/doc/filters.texi > > @@ -10150,7 +10150,7 @@ It accepts the following options: > > > > @table @option > > @item object > > -Filepath of the object image, needs to be in gray8. > > +Filepath of the object image. > > > > @item threshold > > Detection threshold, default is 0.5. > > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > > index d7e6579af7..ee6c3f4b45 100644 > > --- a/libavfilter/vf_find_rect.c > > +++ b/libavfilter/vf_find_rect.c > > @@ -28,6 +28,7 @@ > > #include "internal.h" > > > > #include "lavfutils.h" > > +#include "lswsutils.h" > > > > #define MAX_MIPMAPS 5 > > > > @@ -244,6 +245,9 @@ static av_cold int init(AVFilterContext *ctx) > > { > > FOCContext *foc = ctx->priv; > > int ret, i; > > + uint8_t *tmp_data[4] = { NULL }; > > + int tmp_linesize[4], width, height; > > + enum AVPixelFormat pix_fmt; > > > > if (!foc->obj_filename) { > > av_log(ctx, AV_LOG_ERROR, "object filename not set\n"); > > @@ -254,24 +258,37 @@ static av_cold int init(AVFilterContext *ctx) > > if (!foc->obj_frame) > > return AVERROR(ENOMEM); > > > > - if ((ret = ff_load_image(foc->obj_frame->data, > foc->obj_frame->linesize, > > - &foc->obj_frame->width, > &foc->obj_frame->height, > > - &foc->obj_frame->format, > foc->obj_filename, ctx)) < 0) > > - return ret; > > - > > - if (foc->obj_frame->format != AV_PIX_FMT_GRAY8) { > > - av_log(ctx, AV_LOG_ERROR, "object image is not a grayscale > image\n"); > > - return AVERROR(EINVAL); > > - } > > + if ((ret = ff_load_image(tmp_data, tmp_linesize, > > + &width, &height, > > + &pix_fmt, foc->obj_filename, ctx)) < 0) > > + goto error; > > + > > + /* convert object image to gray8 format with same width and height > */ > > + foc->obj_frame->format = AV_PIX_FMT_GRAY8; > > + foc->obj_frame->width = width; > > + foc->obj_frame->height = height; > > + if ((ret = ff_scale_image(foc->obj_frame->data, > foc->obj_frame->linesize, > > + foc->obj_frame->width, foc->obj_frame->height, > foc->obj_frame->format, > > + tmp_data, tmp_linesize, width, height, pix_fmt, > ctx)) < 0) > > + goto error; > > + av_freep(&tmp_data[0]); > > Iam not sure i understand what this patch is intending to do > > Before a grayscale image was needed, after the patch anything is accepted > but > only grayscale is used. This is quite unexpected for a user who might > expect > color to be used if color is accpeted. > Generally, doing something that is unexpected by the user is not good > > The goal is make the common user to use it more friendly. When I'm do the testing, I have got some object file isn't gray image, I had to use ffmpeg convert it to gray every time. although I don't care for only gray color is used clearly. How about to add description to explain we use gray color only for the compare now. > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Those who are best at talking, realize last or never when they are wrong. > _______________________________________________ > 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 Sat, Jun 15, 2019 at 06:53:20AM +0800, Lance Wang wrote: > On Sat, Jun 15, 2019 at 3:16 AM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Wed, Jun 12, 2019 at 06:57:29PM +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 | 2 +- > > > libavfilter/vf_find_rect.c | 39 +++++++++++++++++++++++++++----------- > > > 2 files changed, 29 insertions(+), 12 deletions(-) > > > > > > diff --git a/doc/filters.texi b/doc/filters.texi > > > index ec1c7c7591..90c57430a6 100644 > > > --- a/doc/filters.texi > > > +++ b/doc/filters.texi > > > @@ -10150,7 +10150,7 @@ It accepts the following options: > > > > > > @table @option > > > @item object > > > -Filepath of the object image, needs to be in gray8. > > > +Filepath of the object image. > > > > > > @item threshold > > > Detection threshold, default is 0.5. > > > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > > > index d7e6579af7..ee6c3f4b45 100644 > > > --- a/libavfilter/vf_find_rect.c > > > +++ b/libavfilter/vf_find_rect.c > > > @@ -28,6 +28,7 @@ > > > #include "internal.h" > > > > > > #include "lavfutils.h" > > > +#include "lswsutils.h" > > > > > > #define MAX_MIPMAPS 5 > > > > > > @@ -244,6 +245,9 @@ static av_cold int init(AVFilterContext *ctx) > > > { > > > FOCContext *foc = ctx->priv; > > > int ret, i; > > > + uint8_t *tmp_data[4] = { NULL }; > > > + int tmp_linesize[4], width, height; > > > + enum AVPixelFormat pix_fmt; > > > > > > if (!foc->obj_filename) { > > > av_log(ctx, AV_LOG_ERROR, "object filename not set\n"); > > > @@ -254,24 +258,37 @@ static av_cold int init(AVFilterContext *ctx) > > > if (!foc->obj_frame) > > > return AVERROR(ENOMEM); > > > > > > - if ((ret = ff_load_image(foc->obj_frame->data, > > foc->obj_frame->linesize, > > > - &foc->obj_frame->width, > > &foc->obj_frame->height, > > > - &foc->obj_frame->format, > > foc->obj_filename, ctx)) < 0) > > > - return ret; > > > - > > > - if (foc->obj_frame->format != AV_PIX_FMT_GRAY8) { > > > - av_log(ctx, AV_LOG_ERROR, "object image is not a grayscale > > image\n"); > > > - return AVERROR(EINVAL); > > > - } > > > + if ((ret = ff_load_image(tmp_data, tmp_linesize, > > > + &width, &height, > > > + &pix_fmt, foc->obj_filename, ctx)) < 0) > > > + goto error; > > > + > > > + /* convert object image to gray8 format with same width and height > > */ > > > + foc->obj_frame->format = AV_PIX_FMT_GRAY8; > > > + foc->obj_frame->width = width; > > > + foc->obj_frame->height = height; > > > + if ((ret = ff_scale_image(foc->obj_frame->data, > > foc->obj_frame->linesize, > > > + foc->obj_frame->width, foc->obj_frame->height, > > foc->obj_frame->format, > > > + tmp_data, tmp_linesize, width, height, pix_fmt, > > ctx)) < 0) > > > + goto error; > > > + av_freep(&tmp_data[0]); > > > > Iam not sure i understand what this patch is intending to do > > > > Before a grayscale image was needed, after the patch anything is accepted > > but > > only grayscale is used. This is quite unexpected for a user who might > > expect > > color to be used if color is accpeted. > > Generally, doing something that is unexpected by the user is not good > > > > The goal is make the common user to use it more friendly. When I'm do the > testing, I have got some object > file isn't gray image, I had to use ffmpeg convert it to gray every time. > although I don't care for only gray color is used > clearly. > > How about to add description to explain we use gray color only for the > compare now. not sure its best but its better, yes thx [...]
On Sun, Jun 16, 2019 at 6:16 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Jun 15, 2019 at 06:53:20AM +0800, Lance Wang wrote: > > On Sat, Jun 15, 2019 at 3:16 AM Michael Niedermayer > <michael@niedermayer.cc> > > wrote: > > > > > On Wed, Jun 12, 2019 at 06:57:29PM +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 | 2 +- > > > > libavfilter/vf_find_rect.c | 39 > +++++++++++++++++++++++++++----------- > > > > 2 files changed, 29 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/doc/filters.texi b/doc/filters.texi > > > > index ec1c7c7591..90c57430a6 100644 > > > > --- a/doc/filters.texi > > > > +++ b/doc/filters.texi > > > > @@ -10150,7 +10150,7 @@ It accepts the following options: > > > > > > > > @table @option > > > > @item object > > > > -Filepath of the object image, needs to be in gray8. > > > > +Filepath of the object image. > > > > > > > > @item threshold > > > > Detection threshold, default is 0.5. > > > > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > > > > index d7e6579af7..ee6c3f4b45 100644 > > > > --- a/libavfilter/vf_find_rect.c > > > > +++ b/libavfilter/vf_find_rect.c > > > > @@ -28,6 +28,7 @@ > > > > #include "internal.h" > > > > > > > > #include "lavfutils.h" > > > > +#include "lswsutils.h" > > > > > > > > #define MAX_MIPMAPS 5 > > > > > > > > @@ -244,6 +245,9 @@ static av_cold int init(AVFilterContext *ctx) > > > > { > > > > FOCContext *foc = ctx->priv; > > > > int ret, i; > > > > + uint8_t *tmp_data[4] = { NULL }; > > > > + int tmp_linesize[4], width, height; > > > > + enum AVPixelFormat pix_fmt; > > > > > > > > if (!foc->obj_filename) { > > > > av_log(ctx, AV_LOG_ERROR, "object filename not set\n"); > > > > @@ -254,24 +258,37 @@ static av_cold int init(AVFilterContext *ctx) > > > > if (!foc->obj_frame) > > > > return AVERROR(ENOMEM); > > > > > > > > - if ((ret = ff_load_image(foc->obj_frame->data, > > > foc->obj_frame->linesize, > > > > - &foc->obj_frame->width, > > > &foc->obj_frame->height, > > > > - &foc->obj_frame->format, > > > foc->obj_filename, ctx)) < 0) > > > > - return ret; > > > > - > > > > - if (foc->obj_frame->format != AV_PIX_FMT_GRAY8) { > > > > - av_log(ctx, AV_LOG_ERROR, "object image is not a grayscale > > > image\n"); > > > > - return AVERROR(EINVAL); > > > > - } > > > > + if ((ret = ff_load_image(tmp_data, tmp_linesize, > > > > + &width, &height, > > > > + &pix_fmt, foc->obj_filename, ctx)) < 0) > > > > + goto error; > > > > + > > > > + /* convert object image to gray8 format with same width and > height > > > */ > > > > + foc->obj_frame->format = AV_PIX_FMT_GRAY8; > > > > + foc->obj_frame->width = width; > > > > + foc->obj_frame->height = height; > > > > + if ((ret = ff_scale_image(foc->obj_frame->data, > > > foc->obj_frame->linesize, > > > > + foc->obj_frame->width, foc->obj_frame->height, > > > foc->obj_frame->format, > > > > + tmp_data, tmp_linesize, width, height, pix_fmt, > > > ctx)) < 0) > > > > + goto error; > > > > + av_freep(&tmp_data[0]); > > > > > > Iam not sure i understand what this patch is intending to do > > > > > > Before a grayscale image was needed, after the patch anything is > accepted > > > but > > > only grayscale is used. This is quite unexpected for a user who might > > > expect > > > color to be used if color is accpeted. > > > Generally, doing something that is unexpected by the user is not good > > > > > > The goal is make the common user to use it more friendly. When I'm do > the > > testing, I have got some object > > file isn't gray image, I had to use ffmpeg convert it to gray every > time. > > although I don't care for only gray color is used > > clearly. > > > > > How about to add description to explain we use gray color only for the > > compare now. > > not sure its best but its better, yes > > OK, I'll add one description in the filters.texi. > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > It is dangerous to be right in matters on which the established authorities > are wrong. -- Voltaire > _______________________________________________ > 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..90c57430a6 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -10150,7 +10150,7 @@ It accepts the following options: @table @option @item object -Filepath of the object image, needs to be in gray8. +Filepath of the object image. @item threshold Detection threshold, default is 0.5. diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c index d7e6579af7..ee6c3f4b45 100644 --- a/libavfilter/vf_find_rect.c +++ b/libavfilter/vf_find_rect.c @@ -28,6 +28,7 @@ #include "internal.h" #include "lavfutils.h" +#include "lswsutils.h" #define MAX_MIPMAPS 5 @@ -244,6 +245,9 @@ static av_cold int init(AVFilterContext *ctx) { FOCContext *foc = ctx->priv; int ret, i; + uint8_t *tmp_data[4] = { NULL }; + int tmp_linesize[4], width, height; + enum AVPixelFormat pix_fmt; if (!foc->obj_filename) { av_log(ctx, AV_LOG_ERROR, "object filename not set\n"); @@ -254,24 +258,37 @@ static av_cold int init(AVFilterContext *ctx) if (!foc->obj_frame) return AVERROR(ENOMEM); - if ((ret = ff_load_image(foc->obj_frame->data, foc->obj_frame->linesize, - &foc->obj_frame->width, &foc->obj_frame->height, - &foc->obj_frame->format, foc->obj_filename, ctx)) < 0) - return ret; - - if (foc->obj_frame->format != AV_PIX_FMT_GRAY8) { - av_log(ctx, AV_LOG_ERROR, "object image is not a grayscale image\n"); - return AVERROR(EINVAL); - } + if ((ret = ff_load_image(tmp_data, tmp_linesize, + &width, &height, + &pix_fmt, foc->obj_filename, ctx)) < 0) + goto error; + + /* convert object image to gray8 format with same width and height */ + foc->obj_frame->format = AV_PIX_FMT_GRAY8; + foc->obj_frame->width = width; + foc->obj_frame->height = height; + if ((ret = ff_scale_image(foc->obj_frame->data, foc->obj_frame->linesize, + foc->obj_frame->width, foc->obj_frame->height, foc->obj_frame->format, + tmp_data, tmp_linesize, width, height, pix_fmt, ctx)) < 0) + goto error; + av_freep(&tmp_data[0]); foc->needle_frame[0] = av_frame_clone(foc->obj_frame); for (i = 1; i < foc->mipmaps; i++) { foc->needle_frame[i] = downscale(foc->needle_frame[i-1]); - if (!foc->needle_frame[i]) - return AVERROR(ENOMEM); + if (!foc->needle_frame[i]) { + ret = AVERROR(ENOMEM); + goto error; + } } return 0; +error: + av_freep(&tmp_data[0]); + if (foc->obj_frame) + av_freep(&foc->obj_frame->data[0]); + av_frame_free(&foc->obj_frame); + return ret; } static const AVFilterPad foc_inputs[] = {