On 08/05/18 03:35, Xiang, Haihao wrote: > On Mon, 2018-05-07 at 21:46 +0100, Mark Thompson wrote: >> On 04/05/18 15:41, Haihao Xiang wrote: >>> Otherwise it might use unitialized last_pic in av_assert0(last_pic) >>> >>> Signed-off-by: Haihao Xiang <haihao.xi...@intel.com> >>> --- >>> libavcodec/vaapi_encode.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c >>> index 36c85a3815..141e50c8ad 100644 >>> --- a/libavcodec/vaapi_encode.c >>> +++ b/libavcodec/vaapi_encode.c >>> @@ -760,7 +760,7 @@ fail: >>> static int vaapi_encode_truncate_gop(AVCodecContext *avctx) >>> { >>> VAAPIEncodeContext *ctx = avctx->priv_data; >>> - VAAPIEncodePicture *pic, *last_pic, *next; >>> + VAAPIEncodePicture *pic, *last_pic = NULL, *next; >>> >>> // Find the last picture we actually have input for. >>> for (pic = ctx->pic_start; pic; pic = pic->next) { >>> >> >> How do you hit this? last_pic should always get set. >> > > It was reported by some static analysis tools, but I agree with you that > last_pic should get set in the code path, however if we consider > vaapi_encode_truncate_gop() only, last_pic will be uninitialized if ctx- >> pic_start is non-NULL but ctx->pic_start->input_available is not set. How >> about > to add av_assert0(!ctx->pic_start || ctx->pic_start->input_available) before > the > for () statement and remove av_assert0(last_pic)?
I think you mean "av_assert0(ctx->pic_start && ctx->pic_start->input_available)"? But yes, that would be sensible. I guess it's the assert on last_pic which is confusing the static analysis, since it kindof implies that it could be null. - Mark (I'm currently looking at refactoring all of this logic to assign picture types at encode time rather than input time - the current method makes it very hard to implement more complex reference structures.) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel