On 2/4/2018 11:05 PM, James Almer wrote: > On 2/4/2018 10:33 PM, Carl Eugen Hoyos wrote: >> Hi! >> >> OOM is unlikely as a failure for avfilter_graph_alloc_filter(), the >> patch avoids a surprising error if a filter was not found. >> >> Please comment, Carl Eugen >> >> >> 0001-lavfi-avfiltergraph-Do-not-return-ENOMEM-if-filterch.patch >> >> >> From 6587726a5e96570bb54e49ccf0b7fd6d94b929c8 Mon Sep 17 00:00:00 2001 >> From: Carl Eugen Hoyos <ceffm...@gmail.com> >> Date: Mon, 5 Feb 2018 01:43:08 +0100 >> Subject: [PATCH] lavfi/avfiltergraph: Do not return ENOMEM if filterchain >> init failed. >> >> A more likely reason is that the filter was not found. >> --- >> libavfilter/avfiltergraph.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c >> index 4cc6892..7ccd895 100644 >> --- a/libavfilter/avfiltergraph.c >> +++ b/libavfilter/avfiltergraph.c >> @@ -147,7 +147,7 @@ int avfilter_graph_create_filter(AVFilterContext >> **filt_ctx, const AVFilter *fil >> >> *filt_ctx = avfilter_graph_alloc_filter(graph_ctx, filt, name); >> if (!*filt_ctx) >> - return AVERROR(ENOMEM); >> + return -1; > > -1 is not acceptable for a public function that states it returns an > AVERROR value on failure. > If the issue is that avfilter_graph_alloc_filter() may fail because of > both OOM and an invalid value for filter, then I'm more inclined to use > AVERROR_UNKNOWN here instead. > > That being said, based on the above the return value of > avfilter_graph_alloc_filter() should have never been an AVFilterContext, > but an int instead. > Maybe a new function that returns a proper AVERROR value should be > added. AVERROR(ENOMEM), AVERROR(EINVAL), AVERROR_FILTER_NOT_FOUND, or > whatever corresponds depending on the type of failure.
Of course, assuming the actual reason of failure in avfilter_graph_alloc_filter is important only in avfilter_graph_create_filter and not really in any other situation, then an internal ff_graph_alloc_filter function could be added instead of a new public function for the above purpose. > >> >> ret = avfilter_init_str(*filt_ctx, args); >> if (ret < 0) >> -- 1.7.10.4 >> >> >> >> _______________________________________________ >> 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