Re: [FFmpeg-devel] [PATCH, v4 1/2] lavf/qsvvpp:allocate continuous memory

2019-06-18 Thread Fu, Linjie
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

2019-06-18 Thread Max Dmitrichenko
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,
>_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

2019-06-18 Thread Linjie Fu
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,
   _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".