| Message ID | 20200823100414.154931-1-george@nsup.org |
|---|---|
| State | Accepted |
| Commit | fe964d80fec17f043763405f5804f397279d6b27 |
| Headers | show |
| Series | [FFmpeg-devel] lavf/formats: more logical testing of inputs and outputs. | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Nicolas George: > ff_set_common_formats() is currently only called after > graph_check_validity(), guaranteeing that inputs and outputs > are connected. > If we want to support configuring partially-connected graphs, > we will have a lot of redesign to do anyway. > > Fix CID 1466262 / 1466263. > > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/formats.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/formats.c b/libavfilter/formats.c > index d2edf832e9..dbf9250cda 100644 > --- a/libavfilter/formats.c > +++ b/libavfilter/formats.c > @@ -606,8 +606,8 @@ int ff_set_common_formats(AVFilterContext *ctx, AVFilterFormats *formats) > int ff_default_query_formats(AVFilterContext *ctx) > { > int ret; > - enum AVMediaType type = ctx->inputs && ctx->inputs [0] ? ctx->inputs [0]->type : > - ctx->outputs && ctx->outputs[0] ? ctx->outputs[0]->type : > + enum AVMediaType type = ctx->nb_inputs ? ctx->inputs [0]->type : > + ctx->nb_outputs ? ctx->outputs[0]->type : > AVMEDIA_TYPE_VIDEO; > > ret = ff_set_common_formats(ctx, ff_all_formats(type)); > IIRC lavf = libavformat, lavfi = libavfilter. But I might be wrong. The change itself is ok; I wondered whether I should send a patch for this, but I dislike patches designed to make an analyzer happy. But given that it is possible for ctx->inputs to be different from NULL even if ctx->nb_inputs is zero (namely if ctx->inputs = av_malloc_array(ctx->nb_inputs, sizeof...) has been used) the new check is more correct and not only for Coverity. Notice that these issues are actually not new: They are the same as 1452428 and 1452429 and 242ba4d74cc95aa78528e4496de7cc63816a877b just made Coverity detect this as new issues. - Andreas
Andreas Rheinhardt (12020-08-23): > IIRC lavf = libavformat, lavfi = libavfilter. But I might be wrong. Fixed and pushed. > The change itself is ok; I wondered whether I should send a patch for > this, but I dislike patches designed to make an analyzer happy. But > given that it is possible for ctx->inputs to be different from NULL even > if ctx->nb_inputs is zero (namely if ctx->inputs = > av_malloc_array(ctx->nb_inputs, sizeof...) has been used) the new check > is more correct and not only for Coverity. > Notice that these issues are actually not new: They are the same as > 1452428 and 1452429 and 242ba4d74cc95aa78528e4496de7cc63816a877b just > made Coverity detect this as new issues. I agree with you. I would not have made the change if it was just to make Coverity happy, but in this instance it was right: these tests were misleading, remnants of early hours of lavfi when we considered things that were abandoned later. Regards,
diff --git a/libavfilter/formats.c b/libavfilter/formats.c index d2edf832e9..dbf9250cda 100644 --- a/libavfilter/formats.c +++ b/libavfilter/formats.c @@ -606,8 +606,8 @@ int ff_set_common_formats(AVFilterContext *ctx, AVFilterFormats *formats) int ff_default_query_formats(AVFilterContext *ctx) { int ret; - enum AVMediaType type = ctx->inputs && ctx->inputs [0] ? ctx->inputs [0]->type : - ctx->outputs && ctx->outputs[0] ? ctx->outputs[0]->type : + enum AVMediaType type = ctx->nb_inputs ? ctx->inputs [0]->type : + ctx->nb_outputs ? ctx->outputs[0]->type : AVMEDIA_TYPE_VIDEO; ret = ff_set_common_formats(ctx, ff_all_formats(type));
ff_set_common_formats() is currently only called after graph_check_validity(), guaranteeing that inputs and outputs are connected. If we want to support configuring partially-connected graphs, we will have a lot of redesign to do anyway. Fix CID 1466262 / 1466263. Signed-off-by: Nicolas George <george@nsup.org> --- libavfilter/formats.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)