[FFmpeg-devel,PATCHv2] avcodec/v4l2: set sizeimage param for non-raw buffers [fixes #6716]

Submitted by Jorge Ramirez-Ortiz on Oct. 4, 2017, 4:50 p.m.

Details

Message ID 1507135831-7715-1-git-send-email-jorge.ramirez-ortiz@linaro.org
State New
Headers show

Commit Message

Jorge Ramirez-Ortiz Oct. 4, 2017, 4:50 p.m.
Some V4L2 drivers fail to allocate buffers when sizeimage is not set
to a max value. This is indeed the case for s5p-mfc [1]

Most drivers should be able to calculate this value from the frame
dimensions and format - or at least have their own default.

However since this work around should not impact those drivers doing
the "right thing" this commit just provides such a default.

The calculations were extracted from the v4l2 driver used to develop
the ffmpeg v4l2_m2m support [2]. See venc.c and vdec.c

[1] linux.git/drivers/media/platform/s5p-mfc
[2] linux.git/drivers/media/platform/qcom/venus/
---
 libavcodec/v4l2_context.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

Comments

Michael Niedermayer Oct. 4, 2017, 6:56 p.m.
On Wed, Oct 04, 2017 at 06:50:31PM +0200, Jorge Ramirez-Ortiz wrote:
> Some V4L2 drivers fail to allocate buffers when sizeimage is not set
> to a max value. This is indeed the case for s5p-mfc [1]
> 
> Most drivers should be able to calculate this value from the frame
> dimensions and format - or at least have their own default.
> 
> However since this work around should not impact those drivers doing
> the "right thing" this commit just provides such a default.
> 
> The calculations were extracted from the v4l2 driver used to develop
> the ffmpeg v4l2_m2m support [2]. See venc.c and vdec.c
> 
> [1] linux.git/drivers/media/platform/s5p-mfc
> [2] linux.git/drivers/media/platform/qcom/venus/
> ---
>  libavcodec/v4l2_context.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> index 297792f..312cde5 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -33,6 +33,8 @@
>  #include "v4l2_fmt.h"
>  #include "v4l2_m2m.h"
>  

> +#define ALIGN(x, a) ((x) + (a - 1)) & (~(a - 1))

the "a" is missing ()
also the whlole expression needs to be protected by a () 

easier yet, you can use FFALIGN()

[...]
Jorge Ramirez-Ortiz Oct. 4, 2017, 6:59 p.m.
On 10/04/2017 08:56 PM, Michael Niedermayer wrote:
> On Wed, Oct 04, 2017 at 06:50:31PM +0200, Jorge Ramirez-Ortiz wrote:
>> Some V4L2 drivers fail to allocate buffers when sizeimage is not set
>> to a max value. This is indeed the case for s5p-mfc [1]
>>
>> Most drivers should be able to calculate this value from the frame
>> dimensions and format - or at least have their own default.
>>
>> However since this work around should not impact those drivers doing
>> the "right thing" this commit just provides such a default.
>>
>> The calculations were extracted from the v4l2 driver used to develop
>> the ffmpeg v4l2_m2m support [2]. See venc.c and vdec.c
>>
>> [1] linux.git/drivers/media/platform/s5p-mfc
>> [2] linux.git/drivers/media/platform/qcom/venus/
>> ---
>>   libavcodec/v4l2_context.c | 30 ++++++++++++++++++++++++++++--
>>   1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
>> index 297792f..312cde5 100644
>> --- a/libavcodec/v4l2_context.c
>> +++ b/libavcodec/v4l2_context.c
>> @@ -33,6 +33,8 @@
>>   #include "v4l2_fmt.h"
>>   #include "v4l2_m2m.h"
>>   
>> +#define ALIGN(x, a) ((x) + (a - 1)) & (~(a - 1))
> the "a" is missing ()
> also the whlole expression needs to be protected by a ()
>
> easier yet, you can use FFALIGN()

ok. thanks.!

Patch hide | download patch | download mbox

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index 297792f..312cde5 100644
--- a/libavcodec/v4l2_context.c
+++ b/libavcodec/v4l2_context.c
@@ -33,6 +33,8 @@ 
 #include "v4l2_fmt.h"
 #include "v4l2_m2m.h"
 
+#define ALIGN(x, a) ((x) + (a - 1)) & (~(a - 1))
+
 struct v4l2_format_update {
     uint32_t v4l2_fmt;
     int update_v4l2;
@@ -90,6 +92,20 @@  static inline int v4l2_type_supported(V4L2Context *ctx)
         ctx->type == V4L2_BUF_TYPE_VIDEO_OUTPUT;
 }
 
+static inline int v4l2_get_framesize_compressed(V4L2Context* ctx, int width, int height)
+{
+    V4L2m2mContext *s = ctx_to_m2mctx(ctx);
+    const int SZ_4K = 0x1000;
+    int size;
+
+    if (av_codec_is_decoder(s->avctx->codec))
+        return ((width * height * 3 / 2) / 2) + 128;
+
+    /* encoder */
+    size = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2 / 2;
+    return ALIGN(size, SZ_4K);
+}
+
 static inline void v4l2_save_to_context(V4L2Context* ctx, struct v4l2_format_update *fmt)
 {
     ctx->format.type = ctx->type;
@@ -101,13 +117,23 @@  static inline void v4l2_save_to_context(V4L2Context* ctx, struct v4l2_format_upd
         /* update the sizes to handle the reconfiguration of the capture stream at runtime */
         ctx->format.fmt.pix_mp.height = ctx->height;
         ctx->format.fmt.pix_mp.width = ctx->width;
-        if (fmt->update_v4l2)
+        if (fmt->update_v4l2) {
             ctx->format.fmt.pix_mp.pixelformat = fmt->v4l2_fmt;
+
+            /* s5p-mfc requires the user to specify a buffer size */
+            ctx->format.fmt.pix_mp.plane_fmt[0].sizeimage =
+                v4l2_get_framesize_compressed(ctx, ctx->width, ctx->height);
+        }
     } else {
         ctx->format.fmt.pix.height = ctx->height;
         ctx->format.fmt.pix.width = ctx->width;
-        if (fmt->update_v4l2)
+        if (fmt->update_v4l2) {
             ctx->format.fmt.pix.pixelformat = fmt->v4l2_fmt;
+
+            /* s5p-mfc requires the user to specify a buffer size */
+            ctx->format.fmt.pix.sizeimage =
+                v4l2_get_framesize_compressed(ctx, ctx->width, ctx->height);
+        }
     }
 }