Em 18-11-2012 07:24, Mauro Carvalho Chehab escreveu:
Em 17-11-2012 08:45, Kirill Smelkov escreveu:
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)
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 */
Was too fast answering it... The comments there should also be dropped, as it doesn't
range anymore to infty.
tpf_default = {.numerator = 1001, .denominator = 30000}; /* NTSC */
#define dprintk(dev, level, fmt, arg...) \
seems OK to me.
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
