| Message ID | 1462378881-16625-1-git-send-email-ismael@iodev.co.uk (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Hans Verkuil |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1axzYb-0006dh-7a; Wed, 04 May 2016 16:21:49 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.76/mailfrontend-6) with esmtp id 1axzYZ-0001x9-3W; Wed, 04 May 2016 18:21:49 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752558AbcEDQVo (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Wed, 4 May 2016 12:21:44 -0400 Received: from iodev.co.uk ([82.211.30.53]:34224 "EHLO iodev.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751230AbcEDQVo (ORCPT <rfc822;linux-media@vger.kernel.org>); Wed, 4 May 2016 12:21:44 -0400 Received: from localhost (host169.186-109-120.telecom.net.ar [186.109.120.169]) by iodev.co.uk (Postfix) with ESMTPSA id E7FA81F0EE; Wed, 4 May 2016 12:20:11 -0400 (EDT) From: Ismael Luceno <ismael@iodev.co.uk> To: linux-media@vger.kernel.org Cc: Hans Verkuil <hverkuil@xs4all.nl>, Andrey Utkin <andrey.utkin@corp.bluecherry.net>, Ismael Luceno <ismael@iodev.co.uk> Subject: [PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB Date: Wed, 4 May 2016 13:21:20 -0300 Message-Id: <1462378881-16625-1-git-send-email-ismael@iodev.co.uk> X-Mailer: git-send-email 2.8.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.5.4.161516 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1300_1399 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, NO_URI_HTTPS 0, SINGLE_URI_IN_BODY 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __SINGLE_URI_TEXT 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NO_WWW 0, __URI_NS , __URI_WITH_PATH 0' |
Commit Message
Ismael Luceno
May 4, 2016, 4:21 p.m. UTC
From: Andrey Utkin <andrey.utkin@corp.bluecherry.net> Such frame size is met in practice. Also report oversized frames. [ismael: Reworked warning and commit message] Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> --- drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Comments
On Wed, May 04, 2016 at 01:21:20PM -0300, Ismael Luceno wrote: > From: Andrey Utkin <andrey.utkin@corp.bluecherry.net> > > Such frame size is met in practice. Also report oversized frames. > > [ismael: Reworked warning and commit message] > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> I object against merging the first part. > --- > drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > index 67a14c4..f98017b 100644 > --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > @@ -33,7 +33,7 @@ > #include "solo6x10-jpeg.h" > > #define MIN_VID_BUFFERS 2 > -#define FRAME_BUF_SIZE (196 * 1024) > +#define FRAME_BUF_SIZE (200 * 1024) Please don't push this. It doesn't matter whether there are 196 or 200 KiB because there happen bigger frames. I don't remember details so I cannot point to all time max frame size. AFAIK this issue appeared on one particular customer installation. I don't monitor it closely right now. I think I have compiled custom package for that setup with FRAME_BUF_SIZE increased much more (maybe 10x?). -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/Mai/2016 00:14, Andrey Utkin wrote: > On Wed, May 04, 2016 at 01:21:20PM -0300, Ismael Luceno wrote: > > From: Andrey Utkin <andrey.utkin@corp.bluecherry.net> > > > > Such frame size is met in practice. Also report oversized frames. > > > > [ismael: Reworked warning and commit message] > > > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> > > I object against merging the first part. > > > --- > > drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > > index 67a14c4..f98017b 100644 > > --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > > +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > > @@ -33,7 +33,7 @@ > > #include "solo6x10-jpeg.h" > > > > #define MIN_VID_BUFFERS 2 > > -#define FRAME_BUF_SIZE (196 * 1024) > > +#define FRAME_BUF_SIZE (200 * 1024) > > Please don't push this. > It doesn't matter whether there are 196 or 200 KiB because there happen > bigger frames. > I don't remember details so I cannot point to all time max frame size. > AFAIK this issue appeared on one particular customer installation. I > don't monitor it closely right now. I think I have compiled custom > package for that setup with FRAME_BUF_SIZE increased much more (maybe > 10x?). I don't quite remember the overscan, but the maximum should be around 1.2MB, so yes. If the QM hasn't been tweaked, then the image must be terrible. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Andrey, Since you are the original author, can you give me your Signed-off-by line? My apologies for the very late reply, it's been very busy lately and I am finally catching up... Thanks! Hans On 05/04/2016 06:21 PM, Ismael Luceno wrote: > From: Andrey Utkin <andrey.utkin@corp.bluecherry.net> > > Such frame size is met in practice. Also report oversized frames. > > [ismael: Reworked warning and commit message] > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> > --- > drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > index 67a14c4..f98017b 100644 > --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c > @@ -33,7 +33,7 @@ > #include "solo6x10-jpeg.h" > > #define MIN_VID_BUFFERS 2 > -#define FRAME_BUF_SIZE (196 * 1024) > +#define FRAME_BUF_SIZE (200 * 1024) > #define MP4_QS 16 > #define DMA_ALIGN 4096 > > @@ -323,8 +323,11 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip, > int i; > int ret; > > - if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) > + if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) { > + dev_warn(&solo_dev->pdev->dev, > + "oversized frame (%d bytes)\n", size); > return -EINVAL; > + } > > solo_enc->desc_count = 1; > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 27, 2016 at 11:12:42AM +0200, Hans Verkuil wrote: > Andrey, > > Since you are the original author, can you give me your Signed-off-by line? No, as increasing buffer size by few kilobytes doesn't change anything. I've increased it from 200 to 204, then found new occurances of the issue, then increased it again and again by few kilobytes. Then I got that this is not a (nice) solution, and have never came back to this. Maybe doubling current buffer size would make users forget about this, but I'm not sure maintainers would be glad with such patch. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/28/16 13:48, Andrey Utkin wrote: > On Mon, Jun 27, 2016 at 11:12:42AM +0200, Hans Verkuil wrote: >> Andrey, >> >> Since you are the original author, can you give me your Signed-off-by line? > > No, as increasing buffer size by few kilobytes doesn't change anything. I've > increased it from 200 to 204, then found new occurances of the issue, > then increased it again and again by few kilobytes. Then I got that this > is not a (nice) solution, and have never came back to this. Maybe > doubling current buffer size would make users forget about this, but I'm > not sure maintainers would be glad with such patch. I don't care. Right now it doesn't work. The cause is that the buffers are too small to handle the worst-case situation. So if doubling the size makes it work, then that's perfectly OK. Memory is cheap these days. If it will fail, then that's much worse than consuming a few meg more. Ideally you can calculate what the worst-case size is, but I expect that to be quite difficult if not impossible. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c index 67a14c4..f98017b 100644 --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c @@ -33,7 +33,7 @@ #include "solo6x10-jpeg.h" #define MIN_VID_BUFFERS 2 -#define FRAME_BUF_SIZE (196 * 1024) +#define FRAME_BUF_SIZE (200 * 1024) #define MP4_QS 16 #define DMA_ALIGN 4096 @@ -323,8 +323,11 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip, int i; int ret; - if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) + if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) { + dev_warn(&solo_dev->pdev->dev, + "oversized frame (%d bytes)\n", size); return -EINVAL; + } solo_enc->desc_count = 1;