Re: [FFmpeg-devel] [PATCH, v4 1/2] lavf/qsvvpp:allocate continuous memory
From: Max Dmitrichenko [mailto:maxim@gmail.com] Sent: Tuesday, June 18, 2019 15:06 To: FFmpeg development discussions and patches Cc: Fu, Linjie Subject: Re: [FFmpeg-devel] [PATCH, v4 1/2] lavf/qsvvpp:allocate continuous memory On Tue, Jun 18, 2019 at 9:52 AM Linjie Fu mailto:linjie...@intel.com>> wrote: Mediasdk calls CMRT to copy from video to system memory and requires memory to be continuously allocated across Y and UV. Add a new path to allocate continuous memory when using system out. Use get_continuous_buffer to arrange data according to pixfmt. it would be good to keep such continuous allocations consistent with encoder's implementation, see https://github.com/FFmpeg/FFmpeg/blob/8f6e65183354d1d402ae80c71cba2759fe152018/libavcodec/qsvenc.c#L1218 if needed, needs to be checked before re-allocation. Thanks for the advice and I agree consistency is important. And I’ve thought about the behavior in qsvenc.c as well to make sure the memory is continuous. However, I got some concerns: 1. av_frame_get_buffer() will introduce plane_padding, and it will lead to un-continuous data[0]/data[1]; And the paddings will lead to wrong offset of U/V, and will cause garbage on chroma filed of image. (when using mirror methods) So I choose to use get_continuous_buffer() which is similar but introduced no paddings. 2. Since here is the first the position to allocate memory for output frame, directly allocate a continuous memory seems to be good IMHO. Thanks, Linjie ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH, v4 1/2] lavf/qsvvpp:allocate continuous memory
On Tue, Jun 18, 2019 at 9:52 AM Linjie Fu wrote: > Mediasdk calls CMRT to copy from video to system memory and requires > memory to be continuously allocated across Y and UV. > > Add a new path to allocate continuous memory when using system out. > Use get_continuous_buffer to arrange data according to pixfmt. > > it would be good to keep such continuous allocations consistent with encoder's implementation, see https://github.com/FFmpeg/FFmpeg/blob/8f6e65183354d1d402ae80c71cba2759fe152018/libavcodec/qsvenc.c#L1218 if needed, needs to be checked before re-allocation. > Signed-off-by: Linjie Fu > --- > libavfilter/qsvvpp.c | 67 > 1 file changed, 62 insertions(+), 5 deletions(-) > > diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c > index 8d5ff2eb65..07fe53573d 100644 > --- a/libavfilter/qsvvpp.c > +++ b/libavfilter/qsvvpp.c > @@ -27,6 +27,7 @@ > #include "libavutil/hwcontext_qsv.h" > #include "libavutil/time.h" > #include "libavutil/pixdesc.h" > +#include "libavutil/imgutils.h" > > #include "internal.h" > #include "qsvvpp.h" > @@ -346,6 +347,57 @@ static QSVFrame *submit_frame(QSVVPPContext *s, > AVFilterLink *inlink, AVFrame *p > return qsv_frame; > } > > +static int get_continuous_buffer(AVFrame *frame, int align) > +{ > +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format); > +int ret, i, padded_height; > + > +if (!desc) > +return AVERROR(EINVAL); > + > +if ((ret = av_image_check_size(frame->width, frame->height, 0, NULL)) > < 0) > +return ret; > + > +if (!frame->linesize[0]) { > +if (align <= 0) > +align = 32; /* STRIDE_ALIGN. Should be av_cpu_max_align() */ > + > +for (i=1; i<=align; i+=i) { > +ret = av_image_fill_linesizes(frame->linesize, frame->format, > + FFALIGN(frame->width, i)); > +if (ret < 0) > +return ret; > +if (!(frame->linesize[0] & (align-1))) > +break; > +} > + > +for (i = 0; i < 4 && frame->linesize[i]; i++) > +frame->linesize[i] = FFALIGN(frame->linesize[i], align); > +} > + > +padded_height = FFALIGN(frame->height, 64); > +if ((ret = av_image_fill_pointers(frame->data, frame->format, > padded_height, > + NULL, frame->linesize)) < 0) > +return ret; > + > +frame->buf[0] = av_buffer_alloc(ret); > +if (!frame->buf[0]) { > +ret = AVERROR(ENOMEM); > +goto fail; > +} > + > +if ((ret = av_image_fill_pointers(frame->data, frame->format, > padded_height, > + frame->buf[0]->data, > frame->linesize)) < 0) > +goto fail; > + > +frame->extended_data = frame->data; > + > +return 0; > +fail: > +av_frame_unref(frame); > +return ret; > +} > + > /* get the output surface */ > static QSVFrame *query_frame(QSVVPPContext *s, AVFilterLink *outlink) > { > @@ -375,15 +427,20 @@ static QSVFrame *query_frame(QSVVPPContext *s, > AVFilterLink *outlink) > out_frame->surface = (mfxFrameSurface1 > *)out_frame->frame->data[3]; > } else { > /* Get a frame with aligned dimensions. > - * Libmfx need system memory being 128x64 aligned */ > -out_frame->frame = ff_get_video_buffer(outlink, > - FFALIGN(outlink->w, 128), > - FFALIGN(outlink->h, 64)); > -if (!out_frame->frame) > + * Libmfx need system memory being 128x64 aligned > + * and continuously allocated across Y and UV */ > +out_frame->frame = av_frame_alloc(); > +if (!out_frame->frame) { > return NULL; > +} > > out_frame->frame->width = outlink->w; > out_frame->frame->height = outlink->h; > +out_frame->frame->format = outlink->format; > + > +ret = get_continuous_buffer(out_frame->frame, 128); > +if (ret < 0) > +return NULL; > > ret = map_frame_to_surface(out_frame->frame, >&out_frame->surface_internal); > -- > 2.17.1 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". regards Max ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH, v4 1/2] lavf/qsvvpp:allocate continuous memory
Mediasdk calls CMRT to copy from video to system memory and requires memory to be continuously allocated across Y and UV. Add a new path to allocate continuous memory when using system out. Use get_continuous_buffer to arrange data according to pixfmt. Signed-off-by: Linjie Fu --- libavfilter/qsvvpp.c | 67 1 file changed, 62 insertions(+), 5 deletions(-) diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c index 8d5ff2eb65..07fe53573d 100644 --- a/libavfilter/qsvvpp.c +++ b/libavfilter/qsvvpp.c @@ -27,6 +27,7 @@ #include "libavutil/hwcontext_qsv.h" #include "libavutil/time.h" #include "libavutil/pixdesc.h" +#include "libavutil/imgutils.h" #include "internal.h" #include "qsvvpp.h" @@ -346,6 +347,57 @@ static QSVFrame *submit_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *p return qsv_frame; } +static int get_continuous_buffer(AVFrame *frame, int align) +{ +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format); +int ret, i, padded_height; + +if (!desc) +return AVERROR(EINVAL); + +if ((ret = av_image_check_size(frame->width, frame->height, 0, NULL)) < 0) +return ret; + +if (!frame->linesize[0]) { +if (align <= 0) +align = 32; /* STRIDE_ALIGN. Should be av_cpu_max_align() */ + +for (i=1; i<=align; i+=i) { +ret = av_image_fill_linesizes(frame->linesize, frame->format, + FFALIGN(frame->width, i)); +if (ret < 0) +return ret; +if (!(frame->linesize[0] & (align-1))) +break; +} + +for (i = 0; i < 4 && frame->linesize[i]; i++) +frame->linesize[i] = FFALIGN(frame->linesize[i], align); +} + +padded_height = FFALIGN(frame->height, 64); +if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height, + NULL, frame->linesize)) < 0) +return ret; + +frame->buf[0] = av_buffer_alloc(ret); +if (!frame->buf[0]) { +ret = AVERROR(ENOMEM); +goto fail; +} + +if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height, + frame->buf[0]->data, frame->linesize)) < 0) +goto fail; + +frame->extended_data = frame->data; + +return 0; +fail: +av_frame_unref(frame); +return ret; +} + /* get the output surface */ static QSVFrame *query_frame(QSVVPPContext *s, AVFilterLink *outlink) { @@ -375,15 +427,20 @@ static QSVFrame *query_frame(QSVVPPContext *s, AVFilterLink *outlink) out_frame->surface = (mfxFrameSurface1 *)out_frame->frame->data[3]; } else { /* Get a frame with aligned dimensions. - * Libmfx need system memory being 128x64 aligned */ -out_frame->frame = ff_get_video_buffer(outlink, - FFALIGN(outlink->w, 128), - FFALIGN(outlink->h, 64)); -if (!out_frame->frame) + * Libmfx need system memory being 128x64 aligned + * and continuously allocated across Y and UV */ +out_frame->frame = av_frame_alloc(); +if (!out_frame->frame) { return NULL; +} out_frame->frame->width = outlink->w; out_frame->frame->height = outlink->h; +out_frame->frame->format = outlink->format; + +ret = get_continuous_buffer(out_frame->frame, 128); +if (ret < 0) +return NULL; ret = map_frame_to_surface(out_frame->frame, &out_frame->surface_internal); -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".