| Message ID | 20190618145144.11949-1-linjie.fu@intel.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jun 18, 2019 at 9:52 AM Linjie Fu <linjie.fu@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. > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > 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-request@ffmpeg.org with subject "unsubscribe". regards Max
From: Max Dmitrichenko [mailto:maxim.d33@gmail.com]
Sent: Tuesday, June 18, 2019 15:06
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Fu, Linjie <linjie.fu@intel.com>
Subject: Re: [FFmpeg-devel] [PATCH, v4 1/2] lavf/qsvvpp:allocate continuous memory
On Tue, Jun 18, 2019 at 9:52 AM Linjie Fu <linjie.fu@intel.com<mailto:linjie.fu@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
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);
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 <linjie.fu@intel.com> --- libavfilter/qsvvpp.c | 67 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 5 deletions(-)