[PATCH] stk-webcam: Don't flip the image by default

[PATCH] stk-webcam: Don't flip the image by default

[Hans de Goede]

Prior to this patch the stk-webcam driver was enabling the vflip and mirror
bits in the sensor by default. Which only is the right thing to do if the
sensor is actually mounted upside down, which it usually is not.

Actually we've received upside down reports for both usb-ids which this
driver supports, one for an "ASUSTeK Computer Inc." "A3H" laptop with
a build in 174f:a311 webcam, and one for an "To Be Filled By O.E.M."
"Z96FM" laptop with a build in 05e1:0501 webcam.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/video/stk-webcam.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
index d427f84..86a0fc5 100644
--- a/drivers/media/video/stk-webcam.c
+++ b/drivers/media/video/stk-webcam.c
@@ -38,13 +38,13 @@
 #include "stk-webcam.h"
 
 
-static bool hflip = 1;
+static bool hflip;
 module_param(hflip, bool, 0444);
-MODULE_PARM_DESC(hflip, "Horizontal image flip (mirror). Defaults to 1");
+MODULE_PARM_DESC(hflip, "Horizontal image flip (mirror). Defaults to 0");
 
-static bool vflip = 1;
+static bool vflip;
 module_param(vflip, bool, 0444);
-MODULE_PARM_DESC(vflip, "Vertical image flip. Defaults to 1");
+MODULE_PARM_DESC(vflip, "Vertical image flip. Defaults to 0");
 
 static int debug;
 module_param(debug, int, 0444);
-- 
1.7.9.3

Re: [PATCH] stk-webcam: Don't flip the image by default

[Jaime Velasco]

Sorry, resending message. Hopefully it gets to the list now. I blame gmail...

2012/4/9 Jaime Velasco <jsagarribay@gmail.com>
>
>
> 2012/4/8 Hans de Goede <hdegoede@redhat.com>
>>
>> Prior to this patch the stk-webcam driver was enabling the vflip and
>> mirror
>> bits in the sensor by default. Which only is the right thing to do if the
>> sensor is actually mounted upside down, which it usually is not.
>>
>> Actually we've received upside down reports for both usb-ids which this
>> driver supports, one for an "ASUSTeK Computer Inc." "A3H" laptop with
>> a build in 174f:a311 webcam, and one for an "To Be Filled By O.E.M."
>> "Z96FM" laptop with a build in 05e1:0501 webcam.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
>
> Hi, I don't know hoy many users of stk-webcam could be, but this will
> surely cause a small regression for them. I agree it seems neater your way,
> but I don't think it makes sense to half-break the driver for a set of users
> in order to fix it for another.
>
> That said, if you feel this is the way to go I won't complain. My old
> laptop broke some years ago and I have not touch nor used this driver since.
>
> Regards,
> Jaime

Re: [PATCH] stk-webcam: Don't flip the image by default

[Hans de Goede]

Hi,

On 04/09/2012 09:27 AM, Jaime Velasco wrote:
>
> 2012/4/8 Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
>
>     Prior to this patch the stk-webcam driver was enabling the vflip and mirror
>     bits in the sensor by default. Which only is the right thing to do if the
>     sensor is actually mounted upside down, which it usually is not.
>
>     Actually we've received upside down reports for both usb-ids which this
>     driver supports, one for an "ASUSTeK Computer Inc." "A3H" laptop with
>     a build in 174f:a311 webcam, and one for an "To Be Filled By O.E.M."
>     "Z96FM" laptop with a build in 05e1:0501 webcam.
>
>     Signed-off-by: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
>
>
> Hi, I don't know hoy many users of stk-webcam could be, but this will surely cause a small regression for them. I agree it seems neater your way, but I don't think it makes sense to half-break the driver for a set of users in order to fix it for another.

I understand where you're coming from, but the vflip/hflip options of the driver
in turn toggle bits in the sensor, which default to no flipping. IOW thse bits
should only be set if a sensor is mounted upside down (or a user explicitly
indicates he wants a flipped image). Likely some of these cameras are found
in Asus laptops, and Asus is well known for mounting some of their laptop
webcam modules upside down, I guess this is where the flip by default behavior
comes from, but that does not make it *right*.

Currently we are getting bug reports from users who have a laptop where the
sensor is mounted the right way up, and they are getting an upside down image...

So either way we are doing the wrong thing for a group of users, and we
are going to need a quirk table in the driver, if we add such a quirk
table I would much rather have it contain entries for laptops which
actually have the camera upside down. Esp. since that is how we are doing
it in all other drivers. It just does not make sense to flip by default, and
then keep a list of non buggy laptops, and change the flipping behavior there ...

Regards,

Hans

Re: [PATCH] stk-webcam: Don't flip the image by default

[Mauro Carvalho Chehab]

Em 09-04-2012 07:31, Hans de Goede escreveu:
> Hi,
> 
> On 04/09/2012 09:27 AM, Jaime Velasco wrote:
>>
>> 2012/4/8 Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
>>
>>     Prior to this patch the stk-webcam driver was enabling the vflip and mirror
>>     bits in the sensor by default. Which only is the right thing to do if the
>>     sensor is actually mounted upside down, which it usually is not.
>>
>>     Actually we've received upside down reports for both usb-ids which this
>>     driver supports, one for an "ASUSTeK Computer Inc." "A3H" laptop with
>>     a build in 174f:a311 webcam, and one for an "To Be Filled By O.E.M."
>>     "Z96FM" laptop with a build in 05e1:0501 webcam.
>>
>>     Signed-off-by: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
>>
>>
>> Hi, I don't know hoy many users of stk-webcam could be, but this will surely cause a small regression for them. I agree it seems neater your way, but I don't think it makes sense to half-break the driver for a set of users in order to fix it for another.
> 
> I understand where you're coming from, but the vflip/hflip options of the driver
> in turn toggle bits in the sensor, which default to no flipping. IOW thse bits
> should only be set if a sensor is mounted upside down (or a user explicitly
> indicates he wants a flipped image). Likely some of these cameras are found
> in Asus laptops, and Asus is well known for mounting some of their laptop
> webcam modules upside down, I guess this is where the flip by default behavior
> comes from, but that does not make it *right*.
> 
> Currently we are getting bug reports from users who have a laptop where the
> sensor is mounted the right way up, and they are getting an upside down image...
> 
> So either way we are doing the wrong thing for a group of users, and we
> are going to need a quirk table in the driver, if we add such a quirk
> table I would much rather have it contain entries for laptops which
> actually have the camera upside down. Esp. since that is how we are doing
> it in all other drivers. It just does not make sense to flip by default, and
> then keep a list of non buggy laptops, and change the flipping behavior there ...

Hi Hans,

While such quirk table doesn't exist there, IMHO, the better would be to change 
the default for this modeprobe parameter to -1 meaning AUTO, where auto would mean
no flip for unknown devices, and flip for the ones at the quirk table.

Btw, if the old default were to flip, we know that, for the original device
used by this driver author, flipping is needed. So, if you know how recognize
it, please add it to a quirk table, to minimize regressions.

As Jaime is the developer that submitted it in the first place, for sure he
can help with this quirk table. 

Regards,
Mauro


> 
> 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