Re: [FFmpeg-devel] [PATCH v3] libavfilter/vf_find_rect: convert the object image to gray8 format instead of failed directly
On Sun, Jun 9, 2019 at 4:38 AM Michael Niedermayer wrote: > On Sat, Jun 08, 2019 at 06:53:58AM +0800, lance.lmw...@gmail.com wrote: > > From: Limin Wang > > > > Signed-off-by: Limin Wang > > --- > > libavfilter/vf_find_rect.c | 40 +++- > > 1 file changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > > index d7e6579..6fe12fe 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 > > > > @@ -235,8 +236,6 @@ static av_cold void uninit(AVFilterContext *ctx) > > av_frame_free(>haystack_frame[i]); > > } > > > > -if (foc->obj_frame) > > -av_freep(>obj_frame->data[0]); > > av_frame_free(>obj_frame); > > } > > > > @@ -244,6 +243,9 @@ static av_cold int init(AVFilterContext *ctx) > > { > > FOCContext *foc = ctx->priv; > > int ret, i; > > +uint8_t *tmp_data[4]; > > +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 +256,36 @@ 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, > > - >obj_frame->width, > >obj_frame->height, > > - >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(_data, tmp_linesize, > > + , , > > + _fmt, foc->obj_filename, ctx)) < 0) > > libavfilter/vf_find_rect.c: In function ‘init’: > libavfilter/vf_find_rect.c:261:30: warning: passing argument 1 of > ‘ff_load_image’ from incompatible pointer type [enabled by default] > > > Should be fixed in the update v4 version patch. Have tested with expected function with below command: ./ffmpeg -i ./input.ts -vf find_rect=./logo.jpg,cover_rect=./cover.jpg:mode=cover output.ts > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > During times of universal deceit, telling the truth becomes a > revolutionary act. -- George Orwell > ___ > 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".
Re: [FFmpeg-devel] [PATCH v3] libavfilter/vf_find_rect: convert the object image to gray8 format instead of failed directly
On Sun, Jun 9, 2019 at 5:41 AM Michael Niedermayer wrote: > On Sat, Jun 08, 2019 at 06:53:58AM +0800, lance.lmw...@gmail.com wrote: > > From: Limin Wang > > > > Signed-off-by: Limin Wang > > --- > > libavfilter/vf_find_rect.c | 40 +++- > > 1 file changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > > index d7e6579..6fe12fe 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 > > > > @@ -235,8 +236,6 @@ static av_cold void uninit(AVFilterContext *ctx) > > av_frame_free(>haystack_frame[i]); > > } > > > > -if (foc->obj_frame) > > -av_freep(>obj_frame->data[0]); > > av_frame_free(>obj_frame); > > } > > this alone will leak > > something like: > foc->obj_frame->buf[0] = > av_buffer_create(foc->obj_frame->data[0], > foc->obj_frame->linesize[0] * > foc->obj_frame->height, > av_buffer_default_free, > NULL, > 0); > > would be needed, but maybe theres a cleaner way to do this > like having a function that does all this in one step and produces a normal > AVFrame. I think more filters could benefit from this if theres nothing > like it yet ... > > I misunderstand by another code review and think the buffer is freed by av_free_frame. I'll revert it back in the update patch. > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Its not that you shouldnt use gotos but rather that you should write > readable code and code with gotos often but not always is less readable > ___ > 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".
Re: [FFmpeg-devel] [PATCH v3] libavfilter/vf_find_rect: convert the object image to gray8 format instead of failed directly
On Sat, Jun 08, 2019 at 06:53:58AM +0800, lance.lmw...@gmail.com wrote: > From: Limin Wang > > Signed-off-by: Limin Wang > --- > libavfilter/vf_find_rect.c | 40 +++- > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > index d7e6579..6fe12fe 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 > > @@ -235,8 +236,6 @@ static av_cold void uninit(AVFilterContext *ctx) > av_frame_free(>haystack_frame[i]); > } > > -if (foc->obj_frame) > -av_freep(>obj_frame->data[0]); > av_frame_free(>obj_frame); > } this alone will leak something like: foc->obj_frame->buf[0] = av_buffer_create(foc->obj_frame->data[0], foc->obj_frame->linesize[0] * foc->obj_frame->height, av_buffer_default_free, NULL, 0); would be needed, but maybe theres a cleaner way to do this like having a function that does all this in one step and produces a normal AVFrame. I think more filters could benefit from this if theres nothing like it yet ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable signature.asc Description: PGP signature ___ 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".
Re: [FFmpeg-devel] [PATCH v3] libavfilter/vf_find_rect: convert the object image to gray8 format instead of failed directly
On Sat, Jun 08, 2019 at 06:53:58AM +0800, lance.lmw...@gmail.com wrote: > From: Limin Wang > > Signed-off-by: Limin Wang > --- > libavfilter/vf_find_rect.c | 40 +++- > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c > index d7e6579..6fe12fe 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 > > @@ -235,8 +236,6 @@ static av_cold void uninit(AVFilterContext *ctx) > av_frame_free(>haystack_frame[i]); > } > > -if (foc->obj_frame) > -av_freep(>obj_frame->data[0]); > av_frame_free(>obj_frame); > } > > @@ -244,6 +243,9 @@ static av_cold int init(AVFilterContext *ctx) > { > FOCContext *foc = ctx->priv; > int ret, i; > +uint8_t *tmp_data[4]; > +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 +256,36 @@ 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, > - >obj_frame->width, >obj_frame->height, > - >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(_data, tmp_linesize, > + , , > + _fmt, foc->obj_filename, ctx)) < 0) libavfilter/vf_find_rect.c: In function ‘init’: libavfilter/vf_find_rect.c:261:30: warning: passing argument 1 of ‘ff_load_image’ from incompatible pointer type [enabled by default] [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB During times of universal deceit, telling the truth becomes a revolutionary act. -- George Orwell signature.asc Description: PGP signature ___ 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".