From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 40422C43381 for ; Thu, 28 Feb 2019 18:02:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EFEB0218D3 for ; Thu, 28 Feb 2019 18:02:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1551376954; bh=J2gP5JHMCqpmpNqAkx7wTkeXrlGnPwKUAxLvhfpg464=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=jnCP18vq9B8s7sZgOhbA5K/U2nmykMCybMbOWV0h/798G64KD9fxUkoYpyn5j9wNK PJQebwbsf1jWDPAiuqeR2r23Tti0CrtzN83/nzJZyOY15KYdwa92P1cJ0rkxmW7dX+ 8pdJOAzSWo+lIG90dOemXXWfOcI3aGlodPZoNOpA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388350AbfB1SCd (ORCPT ); Thu, 28 Feb 2019 13:02:33 -0500 Received: from bombadil.infradead.org ([198.137.202.133]:39570 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731734AbfB1SCd (ORCPT ); Thu, 28 Feb 2019 13:02:33 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To: From:Date:Sender:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=MKa/SdKJSS3wjnLkkXjWf5BgKSDntm6s4wvo/avQUow=; b=R+f3ubOo0mMZAQcFqcqi1QkIk uPUAjfoygBZ9eQ8KZla0aUJ9kxbK4vuP9tXnI3jSZToj1zcdbKA+WH9gt+iO9Db0ZOWs/9HZ2DNTO 4+zJZ1p7G1qA8IHdFXjlwZSgxX413GCZuYSomQL6OHAXS17ZoB/H7nBQmCN3rjiJbEp6SGV+HyDoS OLG4zX1MjAQFNtIWVAlGU/ZNAAV3NrfbewwbkAMLkiCR7Ro5VIFU1s5qjFee2FZ+QeHEAiIRhrMos jn4R06Fe7xx1h63poaowp/YSawWwguMxwntggztKOZvSpMhU67VFlhzW2l5u3J7wwx/O0mJ8PWE16 6m71jKn6A==; Received: from 177.41.113.159.dynamic.adsl.gvt.net.br ([177.41.113.159] helo=coco.lan) by bombadil.infradead.org with esmtpsa (Exim 4.90_1 #2 (Red Hat Linux)) id 1gzQ0t-0001fL-Te; Thu, 28 Feb 2019 18:02:32 +0000 Date: Thu, 28 Feb 2019 15:02:28 -0300 From: Mauro Carvalho Chehab To: Linux Media Mailing List Cc: Mauro Carvalho Chehab , Hans Verkuil , Ezequiel Garcia Subject: Re: [PATCH v3] media: vim2m: better handle cap/out buffers with different sizes Message-ID: <20190228150228.6b6fdcc4@coco.lan> In-Reply-To: <2636657c4de4fcd32ab41a50c7b70445345c8561.1551376758.git.mchehab+samsung@kernel.org> References: <2636657c4de4fcd32ab41a50c7b70445345c8561.1551376758.git.mchehab+samsung@kernel.org> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Em Thu, 28 Feb 2019 14:59:21 -0300 Mauro Carvalho Chehab 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 > --- > 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