Em Thu, 28 Feb 2019 14:59:21 -0300 Mauro Carvalho Chehab <[email protected]> escreveu:
> 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. > > This is a temporary fix. Please ignore this. Forgot to change one line (the one with a comment). Sending version 4. > > A proper fix is to either implement a simple scaler at vim2m, > or to better define the behaviour of M2M transform drivers > at V4L2 API with regards to its capability of scaling the > image or not. > > In any case, such changes would deserve a separate patch > anyway, as it would imply on some behavoral change. > > Also, as we have an actual bug of writing data at wrong > places, let's fix this here, and add a mental note that > we need to properly address it. > > Signed-off-by: Mauro Carvalho Chehab <[email protected]> > --- > drivers/media/platform/vim2m.c | 117 +++++++++++++++++++++++---------- > 1 file changed, 81 insertions(+), 36 deletions(-) > > diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c > index 5157a59aeb58..1c55c47b151a 100644 > --- a/drivers/media/platform/vim2m.c > +++ b/drivers/media/platform/vim2m.c > @@ -267,46 +267,66 @@ static const char *type_name(enum v4l2_buf_type type) > #define CLIP(__color) \ > (u8)(((__color) > 0xff) ? 0xff : (((__color) < 0) ? 0 : (__color))) > > -static void copy_two_pixels(struct vim2m_fmt *in, struct vim2m_fmt *out, > +static int fast_copy_two_pixels(struct vim2m_q_data *q_data_in, > + struct vim2m_q_data *q_data_out, > + u8 **src, u8 **dst, int ypos, bool reverse) > +{ > + int depth = q_data_out->fmt->depth >> 3; > + > + /* Only do fast copy when format and resolution are identical */ > + if (q_data_in->fmt->fourcc != q_data_out->fmt->fourcc || > + q_data_in->width != q_data_out->width || > + q_data_in->height != q_data_out->height) > + return 0; > + > + if (!reverse) { > + memcpy(*dst, *src, depth << 1); > + *src += depth << 1; > + *dst += depth << 1; > + return 1; > + } > + > + /* Copy line at reverse order - YUYV format */ > + if (q_data_in->fmt->fourcc == V4L2_PIX_FMT_YUYV) { > + int u, v, y, y1; > + > + *src -= 2; > + > + y1 = (*src)[0]; /* copy as second point */ > + u = (*src)[1]; > + y = (*src)[2]; /* copy as first point */ > + v = (*src)[3]; > + > + *src -= 2; > + > + *(*dst)++ = y; > + *(*dst)++ = u; > + *(*dst)++ = y1; > + *(*dst)++ = v; > + return 1; > + } > + > + /* copy RGB formats in reverse order */ > + memcpy(*dst, *src, depth); > + memcpy(*dst + depth, *src - depth, depth); > + *src -= depth << 1; > + *dst += depth << 1; > + return 1; > +} > + > +static void copy_two_pixels(struct vim2m_q_data *q_data_in, > + struct vim2m_q_data *q_data_out, > u8 **src, u8 **dst, int ypos, bool reverse) > { > + struct vim2m_fmt *out = q_data_out->fmt; > + struct vim2m_fmt *in = q_data_in->fmt; > u8 _r[2], _g[2], _b[2], *r, *g, *b; > int i, step; > > // If format is the same just copy the data, respecting the width > - if (in->fourcc == out->fourcc) { > - int depth = out->depth >> 3; > - > - if (reverse) { > - if (in->fourcc == V4L2_PIX_FMT_YUYV) { > - int u, v, y, y1; > - > - *src -= 2; > - > - y1 = (*src)[0]; /* copy as second point */ > - u = (*src)[1]; > - y = (*src)[2]; /* copy as first point */ > - v = (*src)[3]; > - > - *src -= 2; > - > - *(*dst)++ = y; > - *(*dst)++ = u; > - *(*dst)++ = y1; > - *(*dst)++ = v; > - return; > - } > - > - memcpy(*dst, *src, depth); > - memcpy(*dst + depth, *src - depth, depth); > - *src -= depth << 1; > - } else { > - memcpy(*dst, *src, depth << 1); > - *src += depth << 1; > - } > - *dst += depth << 1; > - return; > - } > + if (fast_copy_two_pixels(q_data_in, q_data_out, > + src, dst, ypos, reverse)) > + return; > > /* Step 1: read two consecutive pixels from src pointer */ > > @@ -506,7 +526,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); > @@ -516,8 +538,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) { > @@ -526,6 +555,17 @@ static int device_process(struct vim2m_ctx *ctx, > return -EFAULT; > } > > + /* > + * FIXME: instead of cropping the image and zeroing any > + * extra data, the proper behavior is to either scale the > + * data or report that scale is not supported (with depends > + * on some API for such purpose). > + */ > + > + /* 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++; > @@ -547,8 +587,13 @@ static int device_process(struct vim2m_ctx *ctx, > p += bytesperline - (q_data_in->fmt->depth >> 3); > > for (x = 0; x < width >> 1; x++) > - copy_two_pixels(in, out, &p, &p_out, y_out, > + copy_two_pixels(q_data_in, q_data_out, &p, &p_out, > y_out, > ctx->mode & MEM2MEM_HFLIP); > + > + /* Go to the next line at the out buffer*/ > + if (width < q_data_out->width) > + p_out += ((q_data_out->width - width) > + * q_data_out->fmt->depth) >> 3; > } > > return 0; Thanks, Mauro
