| Message ID | 20180504144107.16201-2-haihao.xiang@intel.com |
|---|---|
| State | New |
| Headers | show |
On 04/05/18 15:41, Haihao Xiang wrote: > Otherwise va_rt_format might be unitialized > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > --- > libavutil/hwcontext_vaapi.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > index 7daaa951cc..e59042487d 100644 > --- a/libavutil/hwcontext_vaapi.c > +++ b/libavutil/hwcontext_vaapi.c > @@ -1028,6 +1028,11 @@ static int vaapi_map_from_drm(AVHWFramesContext *src_fc, AVFrame *dst, > va_rt_format = vaapi_format_map[i].rt_format; > } > > + if (i >= FF_ARRAY_ELEMS(vaapi_format_map)) { > + av_log(dst_fc, AV_LOG_ERROR, "No matching VA RT format \n"); > + return AVERROR(EINVAL); > + } > + > buffer_handle = desc->objects[0].fd; > buffer_desc.pixel_format = va_fourcc; > buffer_desc.width = src_fc->width; > How would you hit this case? Every fourcc in vaapi_drm_format_map is also present in vaapi_format_map. - Mark
On Mon, 2018-05-07 at 21:48 +0100, Mark Thompson wrote: > On 04/05/18 15:41, Haihao Xiang wrote: > > Otherwise va_rt_format might be unitialized > > > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > > --- > > libavutil/hwcontext_vaapi.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > > index 7daaa951cc..e59042487d 100644 > > --- a/libavutil/hwcontext_vaapi.c > > +++ b/libavutil/hwcontext_vaapi.c > > @@ -1028,6 +1028,11 @@ static int vaapi_map_from_drm(AVHWFramesContext > > *src_fc, AVFrame *dst, > > va_rt_format = vaapi_format_map[i].rt_format; > > } > > > > + if (i >= FF_ARRAY_ELEMS(vaapi_format_map)) { > > + av_log(dst_fc, AV_LOG_ERROR, "No matching VA RT format \n"); > > + return AVERROR(EINVAL); > > + } > > + > > buffer_handle = desc->objects[0].fd; > > buffer_desc.pixel_format = va_fourcc; > > buffer_desc.width = src_fc->width; > > > > How would you hit this case? Every fourcc in vaapi_drm_format_map is also > present in vaapi_format_map. It is reported by static analysis tool as well. I think adding a check here make s sure the relationship between vaapi_drm_format_map and vaapi_format_map. or how about av_assert0(i < FF_ARRAY_ELEMS(vaapi_format_map)) if you don't want to add the if()? > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 08/05/18 03:53, Xiang, Haihao wrote: > On Mon, 2018-05-07 at 21:48 +0100, Mark Thompson wrote: >> On 04/05/18 15:41, Haihao Xiang wrote: >>> Otherwise va_rt_format might be unitialized >>> >>> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> >>> --- >>> libavutil/hwcontext_vaapi.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c >>> index 7daaa951cc..e59042487d 100644 >>> --- a/libavutil/hwcontext_vaapi.c >>> +++ b/libavutil/hwcontext_vaapi.c >>> @@ -1028,6 +1028,11 @@ static int vaapi_map_from_drm(AVHWFramesContext >>> *src_fc, AVFrame *dst, >>> va_rt_format = vaapi_format_map[i].rt_format; >>> } >>> >>> + if (i >= FF_ARRAY_ELEMS(vaapi_format_map)) { >>> + av_log(dst_fc, AV_LOG_ERROR, "No matching VA RT format \n"); >>> + return AVERROR(EINVAL); >>> + } >>> + >>> buffer_handle = desc->objects[0].fd; >>> buffer_desc.pixel_format = va_fourcc; >>> buffer_desc.width = src_fc->width; >>> >> >> How would you hit this case? Every fourcc in vaapi_drm_format_map is also >> present in vaapi_format_map. > > It is reported by static analysis tool as well. I think adding a check here make > s sure the relationship between vaapi_drm_format_map and vaapi_format_map. or > how about av_assert0(i < FF_ARRAY_ELEMS(vaapi_format_map)) if you don't want to > add the if()? An assert feels better to me - it's our bug if a fourcc in vaapi_drm_format_map isn't in vaapi_format_map, returning an error to the user is not helpful. Thanks, - Mark
On Tue, 2018-05-08 at 22:37 +0100, Mark Thompson wrote: > On 08/05/18 03:53, Xiang, Haihao wrote: > > On Mon, 2018-05-07 at 21:48 +0100, Mark Thompson wrote: > > > On 04/05/18 15:41, Haihao Xiang wrote: > > > > Otherwise va_rt_format might be unitialized > > > > > > > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > > > > --- > > > > libavutil/hwcontext_vaapi.c | 5 +++++ > > > > 1 file changed, 5 insertions(+) > > > > > > > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > > > > index 7daaa951cc..e59042487d 100644 > > > > --- a/libavutil/hwcontext_vaapi.c > > > > +++ b/libavutil/hwcontext_vaapi.c > > > > @@ -1028,6 +1028,11 @@ static int vaapi_map_from_drm(AVHWFramesContext > > > > *src_fc, AVFrame *dst, > > > > va_rt_format = vaapi_format_map[i].rt_format; > > > > } > > > > > > > > + if (i >= FF_ARRAY_ELEMS(vaapi_format_map)) { > > > > + av_log(dst_fc, AV_LOG_ERROR, "No matching VA RT format \n"); > > > > + return AVERROR(EINVAL); > > > > + } > > > > + > > > > buffer_handle = desc->objects[0].fd; > > > > buffer_desc.pixel_format = va_fourcc; > > > > buffer_desc.width = src_fc->width; > > > > > > > > > > How would you hit this case? Every fourcc in vaapi_drm_format_map is also > > > present in vaapi_format_map. > > > > It is reported by static analysis tool as well. I think adding a check here > > make > > s sure the relationship between vaapi_drm_format_map and vaapi_format_map. > > or > > how about av_assert0(i < FF_ARRAY_ELEMS(vaapi_format_map)) if you don't want > > to > > add the if()? > > An assert feels better to me - it's our bug if a fourcc in > vaapi_drm_format_map isn't in vaapi_format_map, returning an error to the user > is not helpful. Agree, I will update the patch. Thanks Haihao > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c index 7daaa951cc..e59042487d 100644 --- a/libavutil/hwcontext_vaapi.c +++ b/libavutil/hwcontext_vaapi.c @@ -1028,6 +1028,11 @@ static int vaapi_map_from_drm(AVHWFramesContext *src_fc, AVFrame *dst, va_rt_format = vaapi_format_map[i].rt_format; } + if (i >= FF_ARRAY_ELEMS(vaapi_format_map)) { + av_log(dst_fc, AV_LOG_ERROR, "No matching VA RT format \n"); + return AVERROR(EINVAL); + } + buffer_handle = desc->objects[0].fd; buffer_desc.pixel_format = va_fourcc; buffer_desc.width = src_fc->width;
Otherwise va_rt_format might be unitialized Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> --- libavutil/hwcontext_vaapi.c | 5 +++++ 1 file changed, 5 insertions(+)