Hi Laurent,
On Mon, Oct 6, 2014 at 4:34 PM, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>> > @@ -311,6 +311,7 @@ static int uvc_parse_format(struct uvc_device *dev,
>> > struct uvc_format_desc *fmtdesc;
>> > struct uvc_frame *frame;
>> > const unsigned char *start = buffer;
>> > + bool force_yuy2_to_y8 = false;
>
> To keep things a bit more generic, how about an unsigned int width_multiplier
> initialized to 1 and set to 2 when the quirk applies ?
[...]
>> > @@ -333,6 +334,22 @@ static int uvc_parse_format(struct uvc_device *dev,
>> >
>> > /* Find the format descriptor from its GUID. */
>> > fmtdesc = uvc_format_by_guid(&buffer[5]);
>> > + format->bpp = buffer[21];
>> > +
>> > + if (dev->quirks & UVC_QUIRK_FORCE_Y8) {
>> > + if (fmtdesc && fmtdesc->fcc == V4L2_PIX_FMT_YUYV
>> > &&
>> > + format->bpp == 16) {
>
> I wonder if that check is really needed, all YUYV formats should have 16bpp.
>
>> > + force_yuy2_to_y8 = true;
>> > + fmtdesc = &uvc_fmts[9];
>
> The hardcoded index here is hair-raising :-) How about something like the
> following instead ?
>
> }
>
> format->bpp = buffer[21];
> +
> + /* Some devices report a format that doesn't match what they
> + * really send.
> + */
> + if (dev->quirks & UVC_QUIRK_FORCE_Y8) {
> + if (format->fcc == V4L2_PIX_FMT_YUYV) {
> + strlcpy(format->name, "Greyscale 8-bit (Y8 )",
> + sizeof(format->name));
> + format->fcc = V4L2_PIX_FMT_GREY;
> + format->bpp = 8;
> + width_multiplier = 2;
> + }
> + }
> +
> if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
> ftype = UVC_VS_FRAME_UNCOMPRESSED;
> } else {
>
> I know it duplicates the format string, but as we're trying to move them to
> the V4L2 core anyway, I don't see that as being a big problem.
[...]
>> > @@ -455,6 +471,8 @@ static int uvc_parse_format(struct uvc_device *dev,
>> > frame->bFrameIndex = buffer[3];
>> > frame->bmCapabilities = buffer[4];
>> > frame->wWidth = get_unaligned_le16(&buffer[5]);
>> > + if (force_yuy2_to_y8)
>> > + frame->wWidth *= 2;
>
> This would become
>
> + frame->wWidth = get_unaligned_le16(&buffer[5])
> + * width_multiplier;
>
> If you're fine with that there's no need to resubmit, I'll modify the patch
> when applying it to my tree.
Thank you, I'm fine with your suggested changes.
Especially the format setting part looks a lot more civilized now.
regards
Philipp
--
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
