| Message ID | 1559163423-22745-1-git-send-email-linjie.fu@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
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@intel.com> > --- > libavcodec/vaapi_encode.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) What driver uses this? > 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? > + else { > + err = av_new_packet(pkt, buf->size); > + ptr = pkt->data; > + } > + > if (err < 0) > goto fail_mapped; > > - memcpy(pkt->data, buf->buf, buf->size); > + memcpy(ptr, buf->buf, buf->size); > + ptr += buf->size; > } > > if (pic->type == PICTURE_TYPE_IDR) > - Mark
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Thursday, May 30, 2019 07:17 > To: ffmpeg-devel@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@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
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); + else { + err = av_new_packet(pkt, buf->size); + ptr = pkt->data; + } + if (err < 0) goto fail_mapped; - memcpy(pkt->data, buf->buf, buf->size); + memcpy(ptr, buf->buf, buf->size); + ptr += buf->size; } if (pic->type == PICTURE_TYPE_IDR)
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@intel.com> --- libavcodec/vaapi_encode.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)