[FFmpeg-devel] [PATCH] lavc/vaapi_encode: grow packet if vaMapBuffer returns multiple buffers

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Thursday, May 30, 2019 07:17
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode: grow packet if
> vaMapBuffer returns multiple buffers
> 
> On 29/05/2019 21:57, Linjie Fu wrote:
> > It seems that VA_CODED_BUF_STATUS_SINGLE_NALU allows driver to
> map
> > buffer for each slice.
> >
> > Currently, assigning new buffer for pkt when multiple buffer returns
> > from vaMapBuffer will cover the previous encoded pkt data and lead
> > to encode issues.
> >
> > Using av_grow_packet to expand pkt if several buffers are returned.
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> >  libavcodec/vaapi_encode.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> What driver uses this?

It was found while enabling VP9 VDENC on ICL with iHD.
vaMapBuffer returns two bufs in the buf_list including a zero-sized buf.

IMHO, it should be fixed in driver level, but I guess ffmpeg should be able to 
cope with the multiple buf situation as well.

https://github.com/intel/media-driver/issues/624
> 
> > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> > index 2dda451..2812237 100644
> > --- a/libavcodec/vaapi_encode.c
> > +++ b/libavcodec/vaapi_encode.c
> > @@ -490,6 +490,7 @@ static int vaapi_encode_output(AVCodecContext
> *avctx,
> >      VACodedBufferSegment *buf_list, *buf;
> >      VAStatus vas;
> >      int err;
> > +    uint8_t *ptr;
> >
> >      err = vaapi_encode_wait(avctx, pic);
> >      if (err < 0)
> > @@ -509,11 +510,18 @@ static int vaapi_encode_output(AVCodecContext
> *avctx,
> >          av_log(avctx, AV_LOG_DEBUG, "Output buffer: %u bytes "
> >                 "(status %08x).\n", buf->size, buf->status);
> >
> > -        err = av_new_packet(pkt, buf->size);
> > +        if (pkt->size)
> > +            err = av_grow_packet(pkt, buf->size);
> 
> av_grow_packet() can reallocate the buffer, which will change its address.
> 
> To avoid repeated new allocations and copies, perhaps it would be better to
> iterate through the list first to find out how large the single packet needs to
> be?

Avoiding repeated memcpy and reallocation sounds better to me.
Thanks Mark.

- Linjie