| Message ID | tencent_DB40CD28A00E4DF0CFFAD50CA745B8E91607@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/vf_delogo: remove duplicated code | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Ping. > On Sep 16, 2020, at 1:09 PM, quinkblack@foxmail.com wrote: > > From: Zhao Zhili <quinkblack@foxmail.com> > > 1. Remove the modification of x, y, w and h parameters since they > are reset by filter_frame. > 2. config_input leads to error out when logo area is outside of the > frame, while filter_frame fix the parameters automatically. Make > config_input don't return error to keep the logic consistent. > --- > libavfilter/vf_delogo.c | 39 +++++++++++++++------------------------ > 1 file changed, 15 insertions(+), 24 deletions(-) > > diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c > index 6069c30163..39f06512fa 100644 > --- a/libavfilter/vf_delogo.c > +++ b/libavfilter/vf_delogo.c > @@ -271,11 +271,6 @@ static av_cold int init(AVFilterContext *ctx) > av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n", > s->x, s->y, s->w, s->h, s->band, s->show); > > - s->w += s->band*2; > - s->h += s->band*2; > - s->x -= s->band; > - s->y -= s->band; > - > return 0; > } > > @@ -284,10 +279,21 @@ static int config_input(AVFilterLink *inlink) > DelogoContext *s = inlink->dst->priv; > > /* Check whether the logo area fits in the frame */ > - if (s->x + (s->band - 1) < 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || > - s->y + (s->band - 1) < 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { > - av_log(s, AV_LOG_ERROR, "Logo area is outside of the frame.\n"); > - return AVERROR(EINVAL); > + if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || > + s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { > + av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," > + " logo(x:%d y:%d w:%d h:%d), frame(%dx%d)," > + " auto set the area inside of the frame." > + " Note: x and y must be 1 at least.\n", > + s->x, s->y, s->w, s->h, inlink->w, inlink->h); > + if (s->x + (s->band - 1) <= 0) > + s->x = 1 + s->band; > + if (s->y + (s->band - 1) <= 0) > + s->y = 1 + s->band; > + if (s->x + s->w - (s->band*2 - 2) > inlink->w) > + s->w = inlink->w - s->x - (s->band*2 - 2); > + if (s->y + s->h - (s->band*2 - 2) > inlink->h) > + s->h = inlink->h - s->y - (s->band*2 - 2); > } > > return 0; > @@ -313,21 +319,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in) > s->w = av_expr_eval(s->w_pexpr, s->var_values, s); > s->h = av_expr_eval(s->h_pexpr, s->var_values, s); > > - if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || > - s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { > - av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," > - " auto set the area inside of the frame\n"); > - } > - > - if (s->x + (s->band - 1) <= 0) > - s->x = 1 + s->band; > - if (s->y + (s->band - 1) <= 0) > - s->y = 1 + s->band; > - if (s->x + s->w - (s->band*2 - 2) > inlink->w) > - s->w = inlink->w - s->x - (s->band*2 - 2); > - if (s->y + s->h - (s->band*2 - 2) > inlink->h) > - s->h = inlink->h - s->y - (s->band*2 - 2); > - > ret = config_input(inlink); > if (ret < 0) { > av_frame_free(&in); > -- > 2.17.1 > >
On Sun, Sep 27, 2020 at 12:38:57AM +0800, Zhao Zhili wrote: > Ping. > > > On Sep 16, 2020, at 1:09 PM, quinkblack@foxmail.com wrote: > > > > From: Zhao Zhili <quinkblack@foxmail.com> > > > > 1. Remove the modification of x, y, w and h parameters since they > > are reset by filter_frame. > > 2. config_input leads to error out when logo area is outside of the > > frame, while filter_frame fix the parameters automatically. Make > > config_input don't return error to keep the logic consistent. > > --- Is this causing overreads? Check with valgrind. > > libavfilter/vf_delogo.c | 39 +++++++++++++++------------------------ > > 1 file changed, 15 insertions(+), 24 deletions(-) > > > > diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c > > index 6069c30163..39f06512fa 100644 > > --- a/libavfilter/vf_delogo.c > > +++ b/libavfilter/vf_delogo.c > > @@ -271,11 +271,6 @@ static av_cold int init(AVFilterContext *ctx) > > av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n", > > s->x, s->y, s->w, s->h, s->band, s->show); > > > > - s->w += s->band*2; > > - s->h += s->band*2; > > - s->x -= s->band; > > - s->y -= s->band; > > - > > return 0; > > } > > > > @@ -284,10 +279,21 @@ static int config_input(AVFilterLink *inlink) > > DelogoContext *s = inlink->dst->priv; > > > > /* Check whether the logo area fits in the frame */ > > - if (s->x + (s->band - 1) < 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || > > - s->y + (s->band - 1) < 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { > > - av_log(s, AV_LOG_ERROR, "Logo area is outside of the frame.\n"); > > - return AVERROR(EINVAL); > > + if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || > > + s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { > > + av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," > > + " logo(x:%d y:%d w:%d h:%d), frame(%dx%d)," > > + " auto set the area inside of the frame." > > + " Note: x and y must be 1 at least.\n", > > + s->x, s->y, s->w, s->h, inlink->w, inlink->h); > > + if (s->x + (s->band - 1) <= 0) > > + s->x = 1 + s->band; > > + if (s->y + (s->band - 1) <= 0) > > + s->y = 1 + s->band; > > + if (s->x + s->w - (s->band*2 - 2) > inlink->w) > > + s->w = inlink->w - s->x - (s->band*2 - 2); > > + if (s->y + s->h - (s->band*2 - 2) > inlink->h) > > + s->h = inlink->h - s->y - (s->band*2 - 2); > > } > > > > return 0; > > @@ -313,21 +319,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in) > > s->w = av_expr_eval(s->w_pexpr, s->var_values, s); > > s->h = av_expr_eval(s->h_pexpr, s->var_values, s); > > > > - if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || > > - s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { > > - av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," > > - " auto set the area inside of the frame\n"); > > - } > > - > > - if (s->x + (s->band - 1) <= 0) > > - s->x = 1 + s->band; > > - if (s->y + (s->band - 1) <= 0) > > - s->y = 1 + s->band; > > - if (s->x + s->w - (s->band*2 - 2) > inlink->w) > > - s->w = inlink->w - s->x - (s->band*2 - 2); > > - if (s->y + s->h - (s->band*2 - 2) > inlink->h) > > - s->h = inlink->h - s->y - (s->band*2 - 2); > > - > > ret = config_input(inlink); > > if (ret < 0) { > > av_frame_free(&in); > > -- > > 2.17.1 > > > > > > _______________________________________________ > 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 Sep 27, 2020, at 2:02 AM, Paul B Mahol <onemda@gmail.com> wrote: > > On Sun, Sep 27, 2020 at 12:38:57AM +0800, Zhao Zhili wrote: >> Ping. >> >>> On Sep 16, 2020, at 1:09 PM, quinkblack@foxmail.com wrote: >>> >>> From: Zhao Zhili <quinkblack@foxmail.com> >>> >>> 1. Remove the modification of x, y, w and h parameters since they >>> are reset by filter_frame. >>> 2. config_input leads to error out when logo area is outside of the >>> frame, while filter_frame fix the parameters automatically. Make >>> config_input don't return error to keep the logic consistent. >>> --- > > Is this causing overreads? Check with valgrind. The patch doesn’t add something new. I double checked with valgrind and asan which didn’t show overreads. > >>> libavfilter/vf_delogo.c | 39 +++++++++++++++------------------------ >>> 1 file changed, 15 insertions(+), 24 deletions(-) >>> >>> diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c >>> index 6069c30163..39f06512fa 100644 >>> --- a/libavfilter/vf_delogo.c >>> +++ b/libavfilter/vf_delogo.c >>> @@ -271,11 +271,6 @@ static av_cold int init(AVFilterContext *ctx) >>> av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n", >>> s->x, s->y, s->w, s->h, s->band, s->show); >>> >>> - s->w += s->band*2; >>> - s->h += s->band*2; >>> - s->x -= s->band; >>> - s->y -= s->band; >>> - >>> return 0; >>> } >>> >>> @@ -284,10 +279,21 @@ static int config_input(AVFilterLink *inlink) >>> DelogoContext *s = inlink->dst->priv; >>> >>> /* Check whether the logo area fits in the frame */ >>> - if (s->x + (s->band - 1) < 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || >>> - s->y + (s->band - 1) < 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { >>> - av_log(s, AV_LOG_ERROR, "Logo area is outside of the frame.\n"); >>> - return AVERROR(EINVAL); >>> + if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || >>> + s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { >>> + av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," >>> + " logo(x:%d y:%d w:%d h:%d), frame(%dx%d)," >>> + " auto set the area inside of the frame." >>> + " Note: x and y must be 1 at least.\n", >>> + s->x, s->y, s->w, s->h, inlink->w, inlink->h); >>> + if (s->x + (s->band - 1) <= 0) >>> + s->x = 1 + s->band; >>> + if (s->y + (s->band - 1) <= 0) >>> + s->y = 1 + s->band; >>> + if (s->x + s->w - (s->band*2 - 2) > inlink->w) >>> + s->w = inlink->w - s->x - (s->band*2 - 2); >>> + if (s->y + s->h - (s->band*2 - 2) > inlink->h) >>> + s->h = inlink->h - s->y - (s->band*2 - 2); >>> } >>> >>> return 0; >>> @@ -313,21 +319,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in) >>> s->w = av_expr_eval(s->w_pexpr, s->var_values, s); >>> s->h = av_expr_eval(s->h_pexpr, s->var_values, s); >>> >>> - if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || >>> - s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { >>> - av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," >>> - " auto set the area inside of the frame\n"); >>> - } >>> - >>> - if (s->x + (s->band - 1) <= 0) >>> - s->x = 1 + s->band; >>> - if (s->y + (s->band - 1) <= 0) >>> - s->y = 1 + s->band; >>> - if (s->x + s->w - (s->band*2 - 2) > inlink->w) >>> - s->w = inlink->w - s->x - (s->band*2 - 2); >>> - if (s->y + s->h - (s->band*2 - 2) > inlink->h) >>> - s->h = inlink->h - s->y - (s->band*2 - 2); >>> - >>> ret = config_input(inlink); >>> if (ret < 0) { >>> av_frame_free(&in); >>> -- >>> 2.17.1 >>> >>> >> >> _______________________________________________ >> 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". > _______________________________________________ > 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/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c index 6069c30163..39f06512fa 100644 --- a/libavfilter/vf_delogo.c +++ b/libavfilter/vf_delogo.c @@ -271,11 +271,6 @@ static av_cold int init(AVFilterContext *ctx) av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n", s->x, s->y, s->w, s->h, s->band, s->show); - s->w += s->band*2; - s->h += s->band*2; - s->x -= s->band; - s->y -= s->band; - return 0; } @@ -284,10 +279,21 @@ static int config_input(AVFilterLink *inlink) DelogoContext *s = inlink->dst->priv; /* Check whether the logo area fits in the frame */ - if (s->x + (s->band - 1) < 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || - s->y + (s->band - 1) < 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { - av_log(s, AV_LOG_ERROR, "Logo area is outside of the frame.\n"); - return AVERROR(EINVAL); + if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || + s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { + av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," + " logo(x:%d y:%d w:%d h:%d), frame(%dx%d)," + " auto set the area inside of the frame." + " Note: x and y must be 1 at least.\n", + s->x, s->y, s->w, s->h, inlink->w, inlink->h); + if (s->x + (s->band - 1) <= 0) + s->x = 1 + s->band; + if (s->y + (s->band - 1) <= 0) + s->y = 1 + s->band; + if (s->x + s->w - (s->band*2 - 2) > inlink->w) + s->w = inlink->w - s->x - (s->band*2 - 2); + if (s->y + s->h - (s->band*2 - 2) > inlink->h) + s->h = inlink->h - s->y - (s->band*2 - 2); } return 0; @@ -313,21 +319,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in) s->w = av_expr_eval(s->w_pexpr, s->var_values, s); s->h = av_expr_eval(s->h_pexpr, s->var_values, s); - if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > inlink->w || - s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > inlink->h) { - av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame," - " auto set the area inside of the frame\n"); - } - - if (s->x + (s->band - 1) <= 0) - s->x = 1 + s->band; - if (s->y + (s->band - 1) <= 0) - s->y = 1 + s->band; - if (s->x + s->w - (s->band*2 - 2) > inlink->w) - s->w = inlink->w - s->x - (s->band*2 - 2); - if (s->y + s->h - (s->band*2 - 2) > inlink->h) - s->h = inlink->h - s->y - (s->band*2 - 2); - ret = config_input(inlink); if (ret < 0) { av_frame_free(&in);
From: Zhao Zhili <quinkblack@foxmail.com> 1. Remove the modification of x, y, w and h parameters since they are reset by filter_frame. 2. config_input leads to error out when logo area is outside of the frame, while filter_frame fix the parameters automatically. Make config_input don't return error to keep the logic consistent. --- libavfilter/vf_delogo.c | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-)