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 */
tpf_default = {.numerator = 1001, .denominator = 30000}; /* NTSC */
#define dprintk(dev, level, fmt, arg...) \
--
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
