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

[wm4]

On Wed, 4 Oct 2017 17:48:25 +0200
Jorge Ramirez-Ortiz <jorge.ramirez-ortiz at linaro.org> wrote:

> On 10/04/2017 11:28 AM, Jorge Ramirez-Ortiz wrote:
> > On 10/04/2017 11:23 AM, wm4 wrote:  
> >> On Wed,  4 Oct 2017 11:17:24 +0200
> >> Jorge Ramirez-Ortiz <jorge.ramirez-ortiz at linaro.org> wrote:
> >>  
> >>> Some V4L2 drivers fail to allocate buffers when sizeimage is not set
> >>> to a max value. This is indeed the case for sp5-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.
> >>>
> >>> [1] linux.git/drivers/media/platform/s5p-mfc
> >>> ---
> >>>   libavcodec/v4l2_context.c | 14 ++++++++++++--
> >>>   1 file changed, 12 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> >>> index 297792f..2707ef5 100644
> >>> --- a/libavcodec/v4l2_context.c
> >>> +++ b/libavcodec/v4l2_context.c
> >>> @@ -92,6 +92,8 @@ static inline int v4l2_type_supported(V4L2Context *ctx)
> >>>     static inline void v4l2_save_to_context(V4L2Context* ctx, struct 
> >>> v4l2_format_update *fmt)
> >>>   {
> >>> +    const int MAX_SIZEIMAGE = 2 * 1024 * 1024;
> >>> +
> >>>       ctx->format.type = ctx->type;
> >>>         if (fmt->update_avfmt)
> >>> @@ -101,13 +103,21 @@ 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 MAX buffer size */
> >>> +            ctx->format.fmt.pix_mp.plane_fmt[0].sizeimage = MAX_SIZEIMAGE;
> >>> +        }
> >>>       } 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 MAX buffer size */
> >>> +            ctx->format.fmt.pix.sizeimage = MAX_SIZEIMAGE;
> >>> +        }
> >>>       }
> >>>   }  
> >> Isn't this something that should be fixed in the driver?  
> >
> > yes but it might take forever and I dont know how many other drivers might 
> > need it.
> >  
> >>
> >> Why 2MB?  
> >
> > no analysis done but seems to be enough to hold an encoded frame. Should it be 
> > any bigger?  
> 
> I could use the calculations below if a generic magic number is a problem:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/qcom/venus/venc.c#n52
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/qcom/venus/vdec.c#n49
> 
> please let me know

Well, I don't think there's any reason why the frame size would be
limited to 2MB. I also can't tell if this is for uncompressed or
compressed frames. For uncompressed frames, you could easily compute a
good guess (the exact size depends on alignment and padding). For
compressed frames it's probably impossible.

If the kernel driver somehow can't be fixed and if this is a show
stopper, it's probably OK if this is done to unbreak it, but it should
at least not break other drivers/files which go beyond the limit.