diff mbox series

[FFmpeg-devel] avfilter/vf_dnn_classify: add result check for av_frame_get_side_data

Message ID 20210507064248.17253-1-liuqi05@kuaishou.com
State Accepted
Commit 7ce0f246f48459a65aedeabce58afdaa988d0eaf
Headers show
Series [FFmpeg-devel] avfilter/vf_dnn_classify: add result check for av_frame_get_side_data | 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 7, 2021, 6:42 a.m. UTC
CID: 1482090
there can return null from av_frame_get_side_data, and will use sd->data
after av_frame_get_side_data, so should check null return value.

Signed-off-by: Steven Liu <liuqi05@kuaishou.com>
---
 libavfilter/vf_dnn_classify.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Guo, Yejun May 7, 2021, 8:11 a.m. UTC | #1
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Steven Liu
> Sent: 2021年5月7日 14:43
> To: ffmpeg-devel@ffmpeg.org
> Cc: Steven Liu <liuqi05@kuaishou.com>
> Subject: [FFmpeg-devel] [PATCH] avfilter/vf_dnn_classify: add result check for
> av_frame_get_side_data
> 
> CID: 1482090

thanks for the patch, what does CID mean?

> there can return null from av_frame_get_side_data, and will use sd->data
> after av_frame_get_side_data, so should check null return value.
> 
> Signed-off-by: Steven Liu <liuqi05@kuaishou.com>
> ---
>  libavfilter/vf_dnn_classify.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavfilter/vf_dnn_classify.c b/libavfilter/vf_dnn_classify.c
> index 18fcd452d0..7900255cfe 100644
> --- a/libavfilter/vf_dnn_classify.c
> +++ b/libavfilter/vf_dnn_classify.c
> @@ -77,6 +77,10 @@ static int dnn_classify_post_proc(AVFrame *frame,
> DNNData *output, uint32_t bbox
>      }
> 
>      sd = av_frame_get_side_data(frame,
> AV_FRAME_DATA_DETECTION_BBOXES);
> +    if (!sd) {
> +        av_log(filter_ctx, AV_LOG_ERROR, "Cannot get side data in
> dnn_classify_post_proc\n");
> +        return -1;
> +    }

The check happens in the backend,
see https://github.com/FFmpeg/FFmpeg/blob/master/libavfilter/dnn/dnn_backend_openvino.c#L536, 
this function will not be invoked if sd is NULL.

anyway, I think it's nice to have another check here.
Steven Liu May 7, 2021, 8:30 a.m. UTC | #2
Guo, Yejun <yejun.guo@intel.com> 于2021年5月7日周五 下午4:11写道:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Steven Liu
> > Sent: 2021年5月7日 14:43
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Steven Liu <liuqi05@kuaishou.com>
> > Subject: [FFmpeg-devel] [PATCH] avfilter/vf_dnn_classify: add result check for
> > av_frame_get_side_data
> >
> > CID: 1482090
>
> thanks for the patch, what does CID mean?
It means Coverity ID :D
Have sent a invite to you yet.
>
> > there can return null from av_frame_get_side_data, and will use sd->data
> > after av_frame_get_side_data, so should check null return value.
> >
> > Signed-off-by: Steven Liu <liuqi05@kuaishou.com>
> > ---
> >  libavfilter/vf_dnn_classify.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/libavfilter/vf_dnn_classify.c b/libavfilter/vf_dnn_classify.c
> > index 18fcd452d0..7900255cfe 100644
> > --- a/libavfilter/vf_dnn_classify.c
> > +++ b/libavfilter/vf_dnn_classify.c
> > @@ -77,6 +77,10 @@ static int dnn_classify_post_proc(AVFrame *frame,
> > DNNData *output, uint32_t bbox
> >      }
> >
> >      sd = av_frame_get_side_data(frame,
> > AV_FRAME_DATA_DETECTION_BBOXES);
> > +    if (!sd) {
> > +        av_log(filter_ctx, AV_LOG_ERROR, "Cannot get side data in
> > dnn_classify_post_proc\n");
> > +        return -1;
> > +    }
>
> The check happens in the backend,
> see https://github.com/FFmpeg/FFmpeg/blob/master/libavfilter/dnn/dnn_backend_openvino.c#L536,
> this function will not be invoked if sd is NULL.
Do you mean need contain_valid_detection_bbox before
av_frame_get_side_data here?


>
> anyway, I think it's nice to have another check here.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Guo, Yejun May 7, 2021, 8:54 a.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Steven Liu
> Sent: 2021年5月7日 16:30
> To: FFmpeg development discussions and patches
> <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avfilter/vf_dnn_classify: add result
> check for av_frame_get_side_data
> 
> Guo, Yejun <yejun.guo@intel.com> 于2021年5月7日周五 下午4:11写道:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Steven Liu
> > > Sent: 2021年5月7日 14:43
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Steven Liu <liuqi05@kuaishou.com>
> > > Subject: [FFmpeg-devel] [PATCH] avfilter/vf_dnn_classify: add result check
> for
> > > av_frame_get_side_data
> > >
> > > CID: 1482090
> >
> > thanks for the patch, what does CID mean?
> It means Coverity ID :D
> Have sent a invite to you yet.

waiting for the invitation, I even checked the Junk folder, still nothing received.

> >
> > > there can return null from av_frame_get_side_data, and will use sd->data
> > > after av_frame_get_side_data, so should check null return value.
> > >
> > > Signed-off-by: Steven Liu <liuqi05@kuaishou.com>
> > > ---
> > >  libavfilter/vf_dnn_classify.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/libavfilter/vf_dnn_classify.c b/libavfilter/vf_dnn_classify.c
> > > index 18fcd452d0..7900255cfe 100644
> > > --- a/libavfilter/vf_dnn_classify.c
> > > +++ b/libavfilter/vf_dnn_classify.c
> > > @@ -77,6 +77,10 @@ static int dnn_classify_post_proc(AVFrame *frame,
> > > DNNData *output, uint32_t bbox
> > >      }
> > >
> > >      sd = av_frame_get_side_data(frame,
> > > AV_FRAME_DATA_DETECTION_BBOXES);
> > > +    if (!sd) {
> > > +        av_log(filter_ctx, AV_LOG_ERROR, "Cannot get side data in
> > > dnn_classify_post_proc\n");
> > > +        return -1;
> > > +    }
> >
> > The check happens in the backend,
> > see
> https://github.com/FFmpeg/FFmpeg/blob/master/libavfilter/dnn/dnn_back
> end_openvino.c#L536,
> > this function will not be invoked if sd is NULL.
> Do you mean need contain_valid_detection_bbox before
> av_frame_get_side_data here?

that function is used within dnn backend, i think you code in this patch is good to me.

> 
> 
> >
> > anyway, I think it's nice to have another check here.
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Guo, Yejun May 11, 2021, 3:02 a.m. UTC | #4
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Guo,
> Yejun
> Sent: 2021年5月7日 16:54
> To: FFmpeg development discussions and patches
> <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avfilter/vf_dnn_classify: add result
> check for av_frame_get_side_data
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Steven Liu
> > Sent: 2021年5月7日 16:30
> > To: FFmpeg development discussions and patches
> > <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] avfilter/vf_dnn_classify: add result
> > check for av_frame_get_side_data
> >
> > Guo, Yejun <yejun.guo@intel.com> 于2021年5月7日周五 下午4:11写
> 道:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Steven Liu
> > > > Sent: 2021年5月7日 14:43
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Cc: Steven Liu <liuqi05@kuaishou.com>
> > > > Subject: [FFmpeg-devel] [PATCH] avfilter/vf_dnn_classify: add result
> check
> > for
> > > > av_frame_get_side_data
> > > >
> > > > CID: 1482090
> > >
> > > thanks for the patch, what does CID mean?
> > It means Coverity ID :D
> > Have sent a invite to you yet.
> 
> waiting for the invitation, I even checked the Junk folder, still nothing
> received.
> 
> > >
> > > > there can return null from av_frame_get_side_data, and will use
> sd->data
> > > > after av_frame_get_side_data, so should check null return value.
> > > >
> > > > Signed-off-by: Steven Liu <liuqi05@kuaishou.com>
> > > > ---
> > > >  libavfilter/vf_dnn_classify.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/libavfilter/vf_dnn_classify.c b/libavfilter/vf_dnn_classify.c
> > > > index 18fcd452d0..7900255cfe 100644
> > > > --- a/libavfilter/vf_dnn_classify.c
> > > > +++ b/libavfilter/vf_dnn_classify.c
> > > > @@ -77,6 +77,10 @@ static int dnn_classify_post_proc(AVFrame
> *frame,
> > > > DNNData *output, uint32_t bbox
> > > >      }
> > > >
> > > >      sd = av_frame_get_side_data(frame,
> > > > AV_FRAME_DATA_DETECTION_BBOXES);
> > > > +    if (!sd) {
> > > > +        av_log(filter_ctx, AV_LOG_ERROR, "Cannot get side data in
> > > > dnn_classify_post_proc\n");
> > > > +        return -1;
> > > > +    }
> > >
> > > The check happens in the backend,
> > > see
> >
> https://github.com/FFmpeg/FFmpeg/blob/master/libavfilter/dnn/dnn_back
> > end_openvino.c#L536,
> > > this function will not be invoked if sd is NULL.
> > Do you mean need contain_valid_detection_bbox before
> > av_frame_get_side_data here?
> 
> that function is used within dnn backend, i think you code in this patch is
> good to me.
> 
pushed
diff mbox series

Patch

diff --git a/libavfilter/vf_dnn_classify.c b/libavfilter/vf_dnn_classify.c
index 18fcd452d0..7900255cfe 100644
--- a/libavfilter/vf_dnn_classify.c
+++ b/libavfilter/vf_dnn_classify.c
@@ -77,6 +77,10 @@  static int dnn_classify_post_proc(AVFrame *frame, DNNData *output, uint32_t bbox
     }
 
     sd = av_frame_get_side_data(frame, AV_FRAME_DATA_DETECTION_BBOXES);
+    if (!sd) {
+        av_log(filter_ctx, AV_LOG_ERROR, "Cannot get side data in dnn_classify_post_proc\n");
+        return -1;
+    }
     header = (AVDetectionBBoxHeader *)sd->data;
 
     if (bbox_index == 0) {