On Tue, 2018-05-08 at 22:34 +0100, Mark Thompson wrote:
> 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)"?
>
No. I meant "av_assert0(!ctx->pic_start || ctx->pic_start->input_available)". I
thought ctx->pic_start might be NULL when vaapi_encode_truncate_gop() is called
in vaapi_encode.c, line 865.
However testing a simple transcode of H265 showed input_image->pict_type is
always AV_PICTURE_TYPE_NONE (a bug?), which means vaapi_encode_truncate_gop() in
vaapi_encode.c, line 865 is never called. This piece of code was added in commit
c667c0979cbc2e04d1d00964b82ac49746caa43c to support forcing IDR frame. How do I
set a forced IDR via AVFrame.pict_type?
> 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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
