From patchwork Sat Nov 17 10:45:09 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kirill Smelkov X-Patchwork-Id: 1758911 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 6FEDA3FC8A for ; Sat, 17 Nov 2012 10:46:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754042Ab2KQKq3 (ORCPT ); Sat, 17 Nov 2012 05:46:29 -0500 Received: from forward4h.mail.yandex.net ([84.201.186.22]:42494 "EHLO forward4h.mail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753747Ab2KQKq2 (ORCPT ); Sat, 17 Nov 2012 05:46:28 -0500 Received: from smtp2h.mail.yandex.net (smtp2h.mail.yandex.net [84.201.187.145]) by forward4h.mail.yandex.net (Yandex) with ESMTP id 66DAB1B21A05; Sat, 17 Nov 2012 14:46:20 +0400 (MSK) Received: from smtp2h.mail.yandex.net (localhost [127.0.0.1]) by smtp2h.mail.yandex.net (Yandex) with ESMTP id A716A17000D2; Sat, 17 Nov 2012 14:46:19 +0400 (MSK) Received: from ip-83-149-3-106.nwgsm.ru (ip-83-149-3-106.nwgsm.ru [83.149.3.106]) by smtp2h.mail.yandex.net (nwsmtp/Yandex) with ESMTP id jUseSXkL-k4sSU8DT; Sat, 17 Nov 2012 14:46:13 +0400 Received: from kirr by mini.zxlink with local (Exim 4.80) (envelope-from ) id 1TZfte-0003UZ-1H; Sat, 17 Nov 2012 14:45:10 +0400 Date: Sat, 17 Nov 2012 14:45:09 +0400 From: Kirill Smelkov To: Mauro Carvalho Chehab Cc: Hans Verkuil , Mauro Carvalho Chehab , kirr@mns.spb.ru, linux-media@vger.kernel.org Subject: Re: [PATCH v4] [media] vivi: Teach it to tune FPS Message-ID: <20121117104509.GA10789@mini.zxlink> References: <1350914084-31618-1-git-send-email-kirr@mns.spb.ru> <201211021542.21944.hverkuil@xs4all.nl> <20121107113001.GA3097@tugrik.mns.mnsspb.ru> <201211161438.09046.hverkuil@xs4all.nl> <50A65FF2.8020801@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <50A65FF2.8020801@redhat.com> Organization: NAVYTUX.SPB.RU User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Fri, Nov 16, 2012 at 01:46:58PM -0200, Mauro Carvalho Chehab wrote: > Em 16-11-2012 11:38, Hans Verkuil escreveu: > >On Wed November 7 2012 12:30:01 Kirill Smelkov wrote: [...] > >>diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c > >>index 37d0af8..5d1b374 100644 > >>--- a/drivers/media/platform/vivi.c > >>+++ b/drivers/media/platform/vivi.c > >>@@ -65,8 +65,11 @@ MODULE_PARM_DESC(vid_limit, "capture memory limit in megabytes"); > >> /* Global font descriptor */ > >> static const u8 *font8x16; > >> > >>-/* default to NTSC timeperframe */ > >>-static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, .denominator = 30000}; > >>+/* timeperframe: min/max and default */ > >>+static const struct v4l2_fract > >>+ tpf_min = {.numerator = 1, .denominator = UINT_MAX}, /* 1/infty */ > >>+ tpf_max = {.numerator = UINT_MAX, .denominator = 1}, /* infty */ > > > >I understand your reasoning here, but I wouldn't go with UINT_MAX here. Something like > >1/1000 tpf (or 1 ms) up to 86400/1 tpf (or once a day). With UINT_MAX I am afraid we > >might hit application errors when they manipulate these values. The shortest time > >between frames is 1 ms anyway. > > > >It's the only comment I have, it looks good otherwise. > > As those will be a arbitrary values, I suggest to declare a macro for it at the > beginning of vivi.c file, with some comment explaining the rationale of the choose, > and what else needs to be changed, if this changes (e. g. less than 1ms would require > changing the image generation logic, to avoid producing frames with equal content). Maybe something like this? (please note, I'm not a good text writer. If this needs adjustment please help me shape the text up) --- 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/platform/vivi.c b/drivers/media/platform/vivi.c index 5d1b374..45b8a81 100644 --- a/drivers/media/platform/vivi.c +++ b/drivers/media/platform/vivi.c @@ -36,6 +36,18 @@ #define VIVI_MODULE_NAME "vivi" +/* Maximum allowed frame rate + * + * Vivi will allow setting timeperframe in [1/FPS_MAX - FPS_MAX/1] range. + * + * Ideally FPS_MAX should be infinity, i.e. practically UINT_MAX, but that + * might hit application errors when they manipulate these values. + * + * Besides, for tpf < 1ms image-generation logic should be changed, to avoid + * producing frames with equal content. + */ +#define FPS_MAX 1000 + #define MAX_WIDTH 1920 #define MAX_HEIGHT 1200 @@ -67,8 +79,8 @@ static const u8 *font8x16; /* timeperframe: min/max and default */ static const struct v4l2_fract - tpf_min = {.numerator = 1, .denominator = UINT_MAX}, /* 1/infty */ - tpf_max = {.numerator = UINT_MAX, .denominator = 1}, /* infty */ + tpf_min = {.numerator = 1, .denominator = FPS_MAX}, /* ~1/infty */ + tpf_max = {.numerator = FPS_MAX, .denominator = 1}, /* ~infty */ tpf_default = {.numerator = 1001, .denominator = 30000}; /* NTSC */ #define dprintk(dev, level, fmt, arg...) \