> From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of Luca > Barbato > Sent: Saturday, July 28, 2018 5:55 PM > To: libav-devel@libav.org > Subject: Re: [libav-devel] [PATCH] qsv: enforcing continuous memory layout > > On 28/07/2018 10:53, maxim_d33 wrote: > > --- > > libavcodec/qsvenc.c | 34 ++++++++++++++++++++++++---------- > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index > > e349a075f..9fb1ae01b 100644 > > --- a/libavcodec/qsvenc.c > > +++ b/libavcodec/qsvenc.c > > @@ -1061,6 +1061,7 @@ static int submit_frame(QSVEncContext *q, > const > > AVFrame *frame, { > > QSVFrame *qf; > > int ret; > > + AVFrame* aligned_frame; > > > > ret = get_free_frame(q, &qf); > > if (ret < 0) > > @@ -1081,22 +1082,35 @@ static int submit_frame(QSVEncContext *q, > const AVFrame *frame, > > qf->surface.Data.MemId = &q->frames_ctx.mids[ret]; > > } > > } else { > > - /* make a copy if the input is not padded as libmfx requires */ > > - if (frame->height & 31 || frame->linesize[0] & (q->width_align - > 1)) { > > This can be kept > > > - qf->frame->height = FFALIGN(frame->height, > q->height_align); > > - qf->frame->width = FFALIGN(frame->width, > q->width_align); > > I think > > > - ret = ff_get_buffer(q->avctx, qf->frame, > AV_GET_BUFFER_FLAG_REF); > > + /* make a copy if > > + - the input is not padded > > + - allocations are not continious for data[0]/data[1] > > + required by libmfx */ > > + if ((frame->data[1] - frame->data[0] != frame->linesize[0] * > FFALIGN(frame->height, q->height_align)) ||
Thanks a lot for this patch, should be useful for http://trac.ffmpeg.org/ticket/5708. IIRC, this is only required for NV12 format. For YUV420p, is it must too? If yes, I guess the best fix is not to making a copy (it will introduce performance drop). Continuous layout is required by spec, not only for qsv. Checked https://www.fourcc.org/pixel-format/yuv-nv12/: "A format in which all Y samples are found first in memory as an array of unsigned char with an even number of lines (possibly with a larger stride for memory alignment), _followed immediately_ by an array of unsigned char containing interleaved Cb and Cr samples (such that if addressed as a little-endian WORD type, Cb would be in the LSBs and Cr would be in the MSBs) with the same total stride as the Y samples. This is the preferred 4:2:0 pixel format." So I think the best fixing it in the initial allocation. > > + (frame->height & 31 || frame->linesize[0] & > (q->width_align - 1))) { > > + aligned_frame = av_frame_alloc(); > > + if (!aligned_frame) > > + return AVERROR(ENOMEM); > > I think you can use qf->frame instead of creating another all the time. > > > + aligned_frame->format = frame->format; > > + aligned_frame->height = FFALIGN(frame->height, > q->height_align); > > + aligned_frame->width = FFALIGN(frame->width, > > + q->width_align); > > + > > + ret = av_frame_get_buffer(aligned_frame,q->width_align); Could this make sure the reallocated aligned_frame is continuous memory? > > if (ret < 0) > > return ret; > > > > - qf->frame->height = frame->height; > > - qf->frame->width = frame->width; > > - ret = av_frame_copy(qf->frame, frame); > > + aligned_frame->height = frame->height; > > + aligned_frame->width = frame->width; > > + > > + ret = av_frame_copy(aligned_frame, frame); > > if (ret < 0) { > > - av_frame_unref(qf->frame); > > + av_frame_unref(aligned_frame); > > return ret; > > } > > + > > + av_frame_free(&qf->frame); > > + qf->frame = aligned_frame; > > } else { > > ret = av_frame_ref(qf->frame, frame); > > if (ret < 0) > > > > lu _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
