On Sun, Nov 1, 2015 at 2:14 PM, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Sat, Oct 31, 2015 at 10:47:54AM -0400, Ganesh Ajjanagadde wrote: >> This function can return ENOMEM that needs to be propagated. >> >> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com> >> --- >> libavfilter/vf_pad.c | 10 ++++++---- >> libavfilter/vf_rotate.c | 3 +-- >> libavfilter/vf_scale.c | 5 +++-- >> libavfilter/vf_zscale.c | 5 +++-- >> 4 files changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/libavfilter/vf_pad.c b/libavfilter/vf_pad.c >> index 63dc6a8..e10d41b 100644 >> --- a/libavfilter/vf_pad.c >> +++ b/libavfilter/vf_pad.c >> @@ -114,9 +114,10 @@ static int config_input(AVFilterLink *inlink) >> var_values[VAR_VSUB] = 1 << s->draw.vsub_max; >> >> /* evaluate width and height */ >> - av_expr_parse_and_eval(&res, (expr = s->w_expr), >> + if ((ret = av_expr_parse_and_eval(&res, (expr = s->w_expr), >> var_names, var_values, >> - NULL, NULL, NULL, NULL, NULL, 0, ctx); >> + NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0) >> + goto eval_fail; >> s->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res; >> if ((ret = av_expr_parse_and_eval(&res, (expr = s->h_expr), >> var_names, var_values, >> @@ -131,9 +132,10 @@ static int config_input(AVFilterLink *inlink) >> s->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res; >> >> /* evaluate x and y */ >> - av_expr_parse_and_eval(&res, (expr = s->x_expr), >> + if ((ret = av_expr_parse_and_eval(&res, (expr = s->x_expr), >> var_names, var_values, >> - NULL, NULL, NULL, NULL, NULL, 0, ctx); >> + NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0) >> + goto eval_fail; >> s->x = var_values[VAR_X] = res; >> if ((ret = av_expr_parse_and_eval(&res, (expr = s->y_expr), >> var_names, var_values, >> diff --git a/libavfilter/vf_rotate.c b/libavfilter/vf_rotate.c >> index f12a103..d5e01e2 100644 >> --- a/libavfilter/vf_rotate.c >> +++ b/libavfilter/vf_rotate.c >> @@ -235,8 +235,7 @@ static int config_props(AVFilterLink *outlink) >> } while (0) >> >> /* evaluate width and height */ >> - av_expr_parse_and_eval(&res, expr = rot->outw_expr_str, var_names, >> rot->var_values, >> - func1_names, func1, NULL, NULL, rot, 0, ctx); >> + SET_SIZE_EXPR(outw, "out_w"); >> rot->var_values[VAR_OUT_W] = rot->var_values[VAR_OW] = res; >> rot->outw = res + 0.5; >> SET_SIZE_EXPR(outh, "out_w"); > > >> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c >> index 1032688..2cf14e0 100644 >> --- a/libavfilter/vf_scale.c >> +++ b/libavfilter/vf_scale.c >> @@ -265,9 +265,10 @@ static int config_props(AVFilterLink *outlink) >> var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h; >> >> /* evaluate width and height */ >> - av_expr_parse_and_eval(&res, (expr = scale->w_expr), >> + if ((ret = av_expr_parse_and_eval(&res, (expr = scale->w_expr), >> var_names, var_values, >> - NULL, NULL, NULL, NULL, NULL, 0, ctx); >> + NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0) >> + goto fail; >> scale->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res; >> if ((ret = av_expr_parse_and_eval(&res, (expr = scale->h_expr), >> var_names, var_values, > > the first evaluation can fail and such failure does not represent an > error for the calling code > > this is needed to support referencing the output parameters, > for example > scale=iw/ih*oh*sar:240 > > here the first evalution of width fails but the 3rd succeeds as > the 2nd made "oh" available > > please make sure the other hunks dont introduce similar issues > in other filters (theres similar such double eval in other filters) > also it would make sense to add comments to each such occurance of > double evaluation so noone else adds a check there by mistake
So if I understand correctly there is both ENOMEM and EINVAL. I think ENOMEM should be a "hard" failure that exits immediately - I doubt that a second evaluation makes sense in such a case. For EINVAL, your point makes sense. This will need a little more careful rework. Thanks. Will repost patches once other two are reviewed. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The worst form of inequality is to try to make unequal things equal. > -- Aristotle > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
