On 2/26/19 6:36 PM, Mauro Carvalho Chehab wrote:
> The vim2m driver doesn't enforce that the capture and output
> buffers would have the same size. Do the right thing if the
> buffers are different, zeroing the buffer before writing,
> ensuring that lines will be aligned and it won't write past
> the buffer area.
I don't really like this. Since the vimc driver doesn't scale it shouldn't
automatically crop either. If you want to crop/compose, then the
selection API should be implemented.
That would be the right approach to allowing capture and output
formats (we're talking formats here, not buffer sizes) with
different resolutions.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@xxxxxxxxxx>
> ---
> drivers/media/platform/vim2m.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
> index 89384f324e25..46e3e096123e 100644
> --- a/drivers/media/platform/vim2m.c
> +++ b/drivers/media/platform/vim2m.c
> @@ -481,7 +481,9 @@ static int device_process(struct vim2m_ctx *ctx,
> struct vim2m_dev *dev = ctx->dev;
> struct vim2m_q_data *q_data_in, *q_data_out;
> u8 *p_in, *p, *p_out;
> - int width, height, bytesperline, x, y, y_out, start, end, step;
> + unsigned int width, height, bytesperline, bytesperline_out;
> + unsigned int x, y, y_out;
> + int start, end, step;
> struct vim2m_fmt *in, *out;
>
> q_data_in = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> @@ -491,8 +493,15 @@ static int device_process(struct vim2m_ctx *ctx,
> bytesperline = (q_data_in->width * q_data_in->fmt->depth) >> 3;
>
> q_data_out = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> + bytesperline_out = (q_data_out->width * q_data_out->fmt->depth) >> 3;
> out = q_data_out->fmt;
>
> + /* Crop to the limits of the destination image */
> + if (width > q_data_out->width)
> + width = q_data_out->width;
> + if (height > q_data_out->height)
> + height = q_data_out->height;
> +
> p_in = vb2_plane_vaddr(&in_vb->vb2_buf, 0);
> p_out = vb2_plane_vaddr(&out_vb->vb2_buf, 0);
> if (!p_in || !p_out) {
> @@ -501,6 +510,10 @@ static int device_process(struct vim2m_ctx *ctx,
> return -EFAULT;
> }
>
> + /* Image size is different. Zero buffer first */
> + if (q_data_in->width != q_data_out->width ||
> + q_data_in->height != q_data_out->height)
> + memset(p_out, 0, q_data_out->sizeimage);
> out_vb->sequence = get_q_data(ctx,
> V4L2_BUF_TYPE_VIDEO_CAPTURE)->sequence++;
> in_vb->sequence = q_data_in->sequence++;
> @@ -524,6 +537,11 @@ static int device_process(struct vim2m_ctx *ctx,
> for (x = 0; x < width >> 1; x++)
> copy_two_pixels(in, out, &p, &p_out, y_out,
> ctx->mode & MEM2MEM_HFLIP);
> +
> + /* Go to the next line at the out buffer*/
Add space after 'buffer'.
> + if (width < q_data_out->width)
> + p_out += ((q_data_out->width - width)
> + * q_data_out->fmt->depth) >> 3;
> }
>
> return 0;
> @@ -977,12 +995,6 @@ static int vim2m_buf_prepare(struct vb2_buffer *vb)
> dprintk(ctx->dev, 2, "type: %s\n", type_name(vb->vb2_queue->type));
>
> q_data = get_q_data(ctx, vb->vb2_queue->type);
> - if (vb2_plane_size(vb, 0) < q_data->sizeimage) {
> - dprintk(ctx->dev, "%s data will not fit into plane (%lu < %lu)\n",
> - __func__, vb2_plane_size(vb, 0), (long)q_data->sizeimage);
> - return -EINVAL;
> - }
> -
As discussed on irc, this can't be removed. It checks if the provided buffer
is large enough for the current format.
Regards,
Hans
> vb2_set_plane_payload(vb, 0, q_data->sizeimage);
>
> return 0;
>
