scale_frame() inconsistently handled the lifetime of `in`. Fixes a possible double free and a possible memory leak. The new code always has `scale_frame` take over ownership of the input frame. I first tried writing this code in a way where the calling code retains ownership, but this is nontrivial due to the presence of the no-op short-circuit condition in which the input frame is directly returned. (As an alternative, we could use av_frame_clone() instead, but I wanted to avoid touching the original behavior in this commit)
Showing the changes to libavfilter (1 of 1 file). Show all
Unified · Split
| @@ -869,17 +869,20 @@ static int scale_field(ScaleContext *scale, AVFrame *dst, AVFrame *src, · 868 unmodified lines | |||
| 869 | return 0; | 869 | return 0; |
| 870 | } | 870 | } |
| 871 | 871 | ||
| 872 | static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) | 872 | /* Takes over ownership of *frame_in, passes ownership of *frame_out to caller */ |
| 873 | static int scale_frame(AVFilterLink *link, AVFrame **frame_in, | ||
| 874 | AVFrame **frame_out) | ||
| 873 | { | 875 | { |
| 874 | AVFilterContext *ctx = link->dst; | 876 | AVFilterContext *ctx = link->dst; |
| 875 | ScaleContext *scale = ctx->priv; | 877 | ScaleContext *scale = ctx->priv; |
| 876 | AVFilterLink *outlink = ctx->outputs[0]; | 878 | AVFilterLink *outlink = ctx->outputs[0]; |
| 877 | AVFrame *out; | 879 | AVFrame *out, *in = *frame_in; |
| 878 | const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format); | 880 | const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format); |
| 879 | char buf[32]; | 881 | char buf[32]; |
| 880 | int ret; | 882 | int ret; |
| 881 | int frame_changed; | 883 | int frame_changed; |
| 882 | 884 | ||
| 885 | *frame_in = NULL; | ||
| 883 | *frame_out = NULL; | 886 | *frame_out = NULL; |
| 884 | if (in->colorspace == AVCOL_SPC_YCGCO) | 887 | if (in->colorspace == AVCOL_SPC_YCGCO) |
| 885 | av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n"); | 888 | av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n"); |
| @@ -922,11 +925,11 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) · 36 unmodified lines | |||
| 922 | 925 | ||
| 923 | ret = scale_parse_expr(ctx, NULL, &scale->w_pexpr, "width", scale->w_expr); | 926 | ret = scale_parse_expr(ctx, NULL, &scale->w_pexpr, "width", scale->w_expr); |
| 924 | if (ret < 0) | 927 | if (ret < 0) |
| 925 | return ret; | 928 | goto err; |
| 926 | 929 | ||
| 927 | ret = scale_parse_expr(ctx, NULL, &scale->h_pexpr, "height", scale->h_expr); | 930 | ret = scale_parse_expr(ctx, NULL, &scale->h_pexpr, "height", scale->h_expr); |
| 928 | if (ret < 0) | 931 | if (ret < 0) |
| 929 | return ret; | 932 | goto err; |
| 930 | } | 933 | } |
| 931 | 934 | ||
| 932 | if (ctx->filter == &ff_vf_scale2ref) { | 935 | if (ctx->filter == &ff_vf_scale2ref) { |
| @@ -957,7 +960,7 @@ FF_ENABLE_DEPRECATION_WARNINGS · 24 unmodified lines | |||
| 957 | link->dst->inputs[0]->sample_aspect_ratio.num = in->sample_aspect_ratio.num; | 960 | link->dst->inputs[0]->sample_aspect_ratio.num = in->sample_aspect_ratio.num; |
| 958 | 961 | ||
| 959 | if ((ret = config_props(outlink)) < 0) | 962 | if ((ret = config_props(outlink)) < 0) |
| 960 | return ret; | 963 | goto err; |
| 961 | } | 964 | } |
| 962 | 965 | ||
| 963 | scale: | 966 | scale: |
| @@ -971,10 +974,9 @@ scale: · 7 unmodified lines | |||
| 971 | 974 | ||
| 972 | out = ff_get_video_buffer(outlink, outlink->w, outlink->h); | 975 | out = ff_get_video_buffer(outlink, outlink->w, outlink->h); |
| 973 | if (!out) { | 976 | if (!out) { |
| 974 | av_frame_free(&in); | 977 | ret = AVERROR(ENOMEM); |
| 975 | return AVERROR(ENOMEM); | 978 | goto err; |
| 976 | } | 979 | } |
| 977 | *frame_out = out; | ||
| 978 | 980 | ||
| 979 | av_frame_copy_props(out, in); | 981 | av_frame_copy_props(out, in); |
| 980 | out->width = outlink->w; | 982 | out->width = outlink->w; |
| @@ -999,9 +1001,12 @@ scale: · 18 unmodified lines | |||
| 999 | ret = sws_scale_frame(scale->sws, out, in); | 1001 | ret = sws_scale_frame(scale->sws, out, in); |
| 1000 | } | 1002 | } |
| 1001 | 1003 | ||
| 1002 | av_frame_free(&in); | ||
| 1003 | if (ret < 0) | 1004 | if (ret < 0) |
| 1004 | av_frame_free(frame_out); | 1005 | av_frame_free(&out); |
| 1006 | *frame_out = out; | ||
| 1007 | |||
| 1008 | err: | ||
| 1009 | av_frame_free(&in); | ||
| 1005 | return ret; | 1010 | return ret; |
| 1006 | } | 1011 | } |
| 1007 | 1012 | ||
| @@ -1058,7 +1063,7 @@ FF_ENABLE_DEPRECATION_WARNINGS · 50 unmodified lines | |||
| 1058 | } | 1063 | } |
| 1059 | } | 1064 | } |
| 1060 | 1065 | ||
| 1061 | ret = scale_frame(ctx->inputs[0], in, &out); | 1066 | ret = scale_frame(ctx->inputs[0], &in, &out); |
| 1062 | if (out) { | 1067 | if (out) { |
| 1063 | out->pts = av_rescale_q(fs->pts, fs->time_base, outlink->time_base); | 1068 | out->pts = av_rescale_q(fs->pts, fs->time_base, outlink->time_base); |
| 1064 | return ff_filter_frame(outlink, out); | 1069 | return ff_filter_frame(outlink, out); |
| @@ -1077,7 +1082,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) · 12 unmodified lines | |||
| 1077 | AVFrame *out; | 1082 | AVFrame *out; |
| 1078 | int ret; | 1083 | int ret; |
| 1079 | 1084 | ||
| 1080 | ret = scale_frame(link, in, &out); | 1085 | ret = scale_frame(link, &in, &out); |
| 1081 | if (out) | 1086 | if (out) |
| 1082 | return ff_filter_frame(outlink, out); | 1087 | return ff_filter_frame(outlink, out); |
| 1083 | 1088 | ||