diff mbox series

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

Message ID 20210514025745.35671-1-liuqi05@kuaishou.com
State New
Headers show
Series [FFmpeg-devel,v2] avfilter/vf_guided: add null pointer check of ref_frame and main_frame | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Steven Liu May 14, 2021, 2:57 a.m. UTC 
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@kuaishou.com>
---
 libavfilter/vf_guided.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andreas Rheinhardt May 14, 2021, 10:57 a.m. UTC | #1 
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@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
diff mbox series

Patch

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);
     }