[FFmpeg-devel] [PATCH v2] avfilter/vf_guided: add null pointer check of ref_frame and main_frame

[Andreas Rheinhardt]

Steven Liu:
> fix CID: 1484785
> check ref_frame and main_frame before use them
> Ignore previous patch please, this should better than that.
> 
> Signed-off-by: Steven Liu <liuqi05 at kuaishou.com>
> ---
>  libavfilter/vf_guided.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_guided.c b/libavfilter/vf_guided.c
> index e7c689e7be..0868b9cd4f 100644
> --- a/libavfilter/vf_guided.c
> +++ b/libavfilter/vf_guided.c
> @@ -334,7 +334,7 @@ static int process_frame(FFFrameSync *fs)
>      }
>      av_frame_copy_props(out_frame, main_frame);
>  
> -    if (ctx->is_disabled || !ref_frame) {
> +    if (ctx->is_disabled && ref_frame && main_frame) {
>          av_frame_copy_props(ref_frame, main_frame);
>      }
>  
> 
1. "Ignore previous patch please, this should better than that." does
not belong in the commit message.
2. Checking for main_frame is unnecessary, as that is always set on
success of ff_framesync_dualinput_get().
3. Checking for ctx->is_disabled should be unnecessary, as this filter
has the AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC (and not
AVFILTER_FLAG_SUPPORT_TIMELINE_INTERNAL) set, which means that this
function is not called if this filter is disabled.
4. We actually do not own ref_frame, so it is doubtful whether we are
allowed to modify it.
5. Why are these properties copied at all? They seem unused.

As you probably already guessed, I looked at this myself, which resulted
in this patchset:
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-May/280293.html (Notice
that I am not very well versed in libavfilter APIs.).

- Andreas