* [PATCH] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
@ 2019-10-02 8:47 Sergey Zakharchenko
2019-10-02 12:00 ` kbuild test robot
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Zakharchenko @ 2019-10-02 8:47 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Mauro Carvalho Chehab, Martin Bodo, Logan,
Peter, Auke Kok, Sergey Zakharchenko, Sergey Zakharchenko
This device does not function correctly in raw mode in kernel
versions validating buffer sizes in bulk mode. It erroneously
announces 16 bits per pixel instead of 12 for NV12 format, so it
needs this quirk to fix computed frame size and avoid legitimate
frames getting discarded.
Signed-off-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com>
---
drivers/media/usb/uvc/uvc_driver.c | 27 +++++++++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 66ee168ddc7e..ffb3bc0992cc 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -446,10 +446,12 @@ static int uvc_parse_format(struct uvc_device *dev,
struct usb_host_interface *alts = intf->cur_altsetting;
struct uvc_format_desc *fmtdesc;
struct uvc_frame *frame;
+ const struct v4l2_format_info *info;
const unsigned char *start = buffer;
unsigned int width_multiplier = 1;
unsigned int interval;
unsigned int i, n;
+ unsigned int div;
u8 ftype;
format->type = buffer[2];
@@ -497,6 +499,18 @@ static int uvc_parse_format(struct uvc_device *dev,
}
}
+ /* Some devices report bpp that doesn't match the format. */
+ if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
+ info = v4l2_format_info(format->fcc);
+ if (info) {
+ div = info->hdiv * info->vdiv;
+ n = info->bpp[i] * div;
+ for (i = 1; i < info->comp_planes; i++)
+ n += info->bpp[i];
+ format->bpp = DIV_ROUND_UP(8 * n, div);
+ }
+ }
+
if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
ftype = UVC_VS_FRAME_UNCOMPRESSED;
} else {
@@ -2384,6 +2398,10 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
.quirks = UVC_QUIRK_FORCE_Y8,
};
+static const struct uvc_device_info uvc_quirk_force_bpp = {
+ .quirks = UVC_QUIRK_FORCE_BPP,
+};
+
#define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks = q}
#define UVC_INFO_META(m) (kernel_ulong_t)&(struct uvc_device_info) \
{.meta_format = m}
@@ -2869,6 +2887,15 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = UVC_INFO_META(V4L2_META_FMT_D4XX) },
+ /* GEO Semiconductor GC6500 */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x29fe,
+ .idProduct = 0x4d53,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_quirk_force_bpp },
/* Generic USB Video Class */
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) },
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) },
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index c7c1baa90dea..24e3d8c647e7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -198,6 +198,7 @@
#define UVC_QUIRK_RESTRICT_FRAME_RATE 0x00000200
#define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
#define UVC_QUIRK_FORCE_Y8 0x00000800
+#define UVC_QUIRK_FORCE_BPP 0x00001000
/* Format flags */
#define UVC_FMT_FLAG_COMPRESSED 0x00000001
--
2.23.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 8:47 [PATCH] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value Sergey Zakharchenko
@ 2019-10-02 12:00 ` kbuild test robot
0 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2019-10-02 12:00 UTC (permalink / raw)
To: Sergey Zakharchenko
Cc: kbuild-all, linux-media, Laurent Pinchart, Mauro Carvalho Chehab,
Martin Bodo, Logan, Peter, Auke Kok, Sergey Zakharchenko,
Sergey Zakharchenko
[-- Attachment #1: Type: text/plain, Size: 11650 bytes --]
Hi Sergey,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[cannot apply to v5.4-rc1 next-20191002]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Sergey-Zakharchenko/media-uvcvideo-Add-a-quirk-to-force-GEO-GC6500-Camera-bits-per-pixel-value/20191002-185359
base: git://linuxtv.org/media_tree.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/media/usb/uvc/uvc_driver.c: In function 'uvc_parse_format.isra.6':
>> drivers/media/usb/uvc/uvc_driver.c:507:18: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized]
n = info->bpp[i] * div;
~~~~~~~~~^~~
vim +/i +507 drivers/media/usb/uvc/uvc_driver.c
436
437 /* ------------------------------------------------------------------------
438 * Descriptors parsing
439 */
440
441 static int uvc_parse_format(struct uvc_device *dev,
442 struct uvc_streaming *streaming, struct uvc_format *format,
443 u32 **intervals, unsigned char *buffer, int buflen)
444 {
445 struct usb_interface *intf = streaming->intf;
446 struct usb_host_interface *alts = intf->cur_altsetting;
447 struct uvc_format_desc *fmtdesc;
448 struct uvc_frame *frame;
449 const struct v4l2_format_info *info;
450 const unsigned char *start = buffer;
451 unsigned int width_multiplier = 1;
452 unsigned int interval;
453 unsigned int i, n;
454 unsigned int div;
455 u8 ftype;
456
457 format->type = buffer[2];
458 format->index = buffer[3];
459
460 switch (buffer[2]) {
461 case UVC_VS_FORMAT_UNCOMPRESSED:
462 case UVC_VS_FORMAT_FRAME_BASED:
463 n = buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED ? 27 : 28;
464 if (buflen < n) {
465 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
466 "interface %d FORMAT error\n",
467 dev->udev->devnum,
468 alts->desc.bInterfaceNumber);
469 return -EINVAL;
470 }
471
472 /* Find the format descriptor from its GUID. */
473 fmtdesc = uvc_format_by_guid(&buffer[5]);
474
475 if (fmtdesc != NULL) {
476 strscpy(format->name, fmtdesc->name,
477 sizeof(format->name));
478 format->fcc = fmtdesc->fcc;
479 } else {
480 uvc_printk(KERN_INFO, "Unknown video format %pUl\n",
481 &buffer[5]);
482 snprintf(format->name, sizeof(format->name), "%pUl\n",
483 &buffer[5]);
484 format->fcc = 0;
485 }
486
487 format->bpp = buffer[21];
488
489 /* Some devices report a format that doesn't match what they
490 * really send.
491 */
492 if (dev->quirks & UVC_QUIRK_FORCE_Y8) {
493 if (format->fcc == V4L2_PIX_FMT_YUYV) {
494 strscpy(format->name, "Greyscale 8-bit (Y8 )",
495 sizeof(format->name));
496 format->fcc = V4L2_PIX_FMT_GREY;
497 format->bpp = 8;
498 width_multiplier = 2;
499 }
500 }
501
502 /* Some devices report bpp that doesn't match the format. */
503 if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
504 info = v4l2_format_info(format->fcc);
505 if (info) {
506 div = info->hdiv * info->vdiv;
> 507 n = info->bpp[i] * div;
508 for (i = 1; i < info->comp_planes; i++)
509 n += info->bpp[i];
510 format->bpp = DIV_ROUND_UP(8 * n, div);
511 }
512 }
513
514 if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
515 ftype = UVC_VS_FRAME_UNCOMPRESSED;
516 } else {
517 ftype = UVC_VS_FRAME_FRAME_BASED;
518 if (buffer[27])
519 format->flags = UVC_FMT_FLAG_COMPRESSED;
520 }
521 break;
522
523 case UVC_VS_FORMAT_MJPEG:
524 if (buflen < 11) {
525 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
526 "interface %d FORMAT error\n",
527 dev->udev->devnum,
528 alts->desc.bInterfaceNumber);
529 return -EINVAL;
530 }
531
532 strscpy(format->name, "MJPEG", sizeof(format->name));
533 format->fcc = V4L2_PIX_FMT_MJPEG;
534 format->flags = UVC_FMT_FLAG_COMPRESSED;
535 format->bpp = 0;
536 ftype = UVC_VS_FRAME_MJPEG;
537 break;
538
539 case UVC_VS_FORMAT_DV:
540 if (buflen < 9) {
541 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
542 "interface %d FORMAT error\n",
543 dev->udev->devnum,
544 alts->desc.bInterfaceNumber);
545 return -EINVAL;
546 }
547
548 switch (buffer[8] & 0x7f) {
549 case 0:
550 strscpy(format->name, "SD-DV", sizeof(format->name));
551 break;
552 case 1:
553 strscpy(format->name, "SDL-DV", sizeof(format->name));
554 break;
555 case 2:
556 strscpy(format->name, "HD-DV", sizeof(format->name));
557 break;
558 default:
559 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
560 "interface %d: unknown DV format %u\n",
561 dev->udev->devnum,
562 alts->desc.bInterfaceNumber, buffer[8]);
563 return -EINVAL;
564 }
565
566 strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
567 sizeof(format->name));
568
569 format->fcc = V4L2_PIX_FMT_DV;
570 format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM;
571 format->bpp = 0;
572 ftype = 0;
573
574 /* Create a dummy frame descriptor. */
575 frame = &format->frame[0];
576 memset(&format->frame[0], 0, sizeof(format->frame[0]));
577 frame->bFrameIntervalType = 1;
578 frame->dwDefaultFrameInterval = 1;
579 frame->dwFrameInterval = *intervals;
580 *(*intervals)++ = 1;
581 format->nframes = 1;
582 break;
583
584 case UVC_VS_FORMAT_MPEG2TS:
585 case UVC_VS_FORMAT_STREAM_BASED:
586 /* Not supported yet. */
587 default:
588 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
589 "interface %d unsupported format %u\n",
590 dev->udev->devnum, alts->desc.bInterfaceNumber,
591 buffer[2]);
592 return -EINVAL;
593 }
594
595 uvc_trace(UVC_TRACE_DESCR, "Found format %s.\n", format->name);
596
597 buflen -= buffer[0];
598 buffer += buffer[0];
599
600 /* Parse the frame descriptors. Only uncompressed, MJPEG and frame
601 * based formats have frame descriptors.
602 */
603 while (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE &&
604 buffer[2] == ftype) {
605 frame = &format->frame[format->nframes];
606 if (ftype != UVC_VS_FRAME_FRAME_BASED)
607 n = buflen > 25 ? buffer[25] : 0;
608 else
609 n = buflen > 21 ? buffer[21] : 0;
610
611 n = n ? n : 3;
612
613 if (buflen < 26 + 4*n) {
614 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
615 "interface %d FRAME error\n", dev->udev->devnum,
616 alts->desc.bInterfaceNumber);
617 return -EINVAL;
618 }
619
620 frame->bFrameIndex = buffer[3];
621 frame->bmCapabilities = buffer[4];
622 frame->wWidth = get_unaligned_le16(&buffer[5])
623 * width_multiplier;
624 frame->wHeight = get_unaligned_le16(&buffer[7]);
625 frame->dwMinBitRate = get_unaligned_le32(&buffer[9]);
626 frame->dwMaxBitRate = get_unaligned_le32(&buffer[13]);
627 if (ftype != UVC_VS_FRAME_FRAME_BASED) {
628 frame->dwMaxVideoFrameBufferSize =
629 get_unaligned_le32(&buffer[17]);
630 frame->dwDefaultFrameInterval =
631 get_unaligned_le32(&buffer[21]);
632 frame->bFrameIntervalType = buffer[25];
633 } else {
634 frame->dwMaxVideoFrameBufferSize = 0;
635 frame->dwDefaultFrameInterval =
636 get_unaligned_le32(&buffer[17]);
637 frame->bFrameIntervalType = buffer[21];
638 }
639 frame->dwFrameInterval = *intervals;
640
641 /* Several UVC chipsets screw up dwMaxVideoFrameBufferSize
642 * completely. Observed behaviours range from setting the
643 * value to 1.1x the actual frame size to hardwiring the
644 * 16 low bits to 0. This results in a higher than necessary
645 * memory usage as well as a wrong image size information. For
646 * uncompressed formats this can be fixed by computing the
647 * value from the frame size.
648 */
649 if (!(format->flags & UVC_FMT_FLAG_COMPRESSED))
650 frame->dwMaxVideoFrameBufferSize = format->bpp
651 * frame->wWidth * frame->wHeight / 8;
652
653 /* Some bogus devices report dwMinFrameInterval equal to
654 * dwMaxFrameInterval and have dwFrameIntervalStep set to
655 * zero. Setting all null intervals to 1 fixes the problem and
656 * some other divisions by zero that could happen.
657 */
658 for (i = 0; i < n; ++i) {
659 interval = get_unaligned_le32(&buffer[26+4*i]);
660 *(*intervals)++ = interval ? interval : 1;
661 }
662
663 /* Make sure that the default frame interval stays between
664 * the boundaries.
665 */
666 n -= frame->bFrameIntervalType ? 1 : 2;
667 frame->dwDefaultFrameInterval =
668 min(frame->dwFrameInterval[n],
669 max(frame->dwFrameInterval[0],
670 frame->dwDefaultFrameInterval));
671
672 if (dev->quirks & UVC_QUIRK_RESTRICT_FRAME_RATE) {
673 frame->bFrameIntervalType = 1;
674 frame->dwFrameInterval[0] =
675 frame->dwDefaultFrameInterval;
676 }
677
678 uvc_trace(UVC_TRACE_DESCR, "- %ux%u (%u.%u fps)\n",
679 frame->wWidth, frame->wHeight,
680 10000000/frame->dwDefaultFrameInterval,
681 (100000000/frame->dwDefaultFrameInterval)%10);
682
683 format->nframes++;
684 buflen -= buffer[0];
685 buffer += buffer[0];
686 }
687
688 if (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE &&
689 buffer[2] == UVC_VS_STILL_IMAGE_FRAME) {
690 buflen -= buffer[0];
691 buffer += buffer[0];
692 }
693
694 if (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE &&
695 buffer[2] == UVC_VS_COLORFORMAT) {
696 if (buflen < 6) {
697 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
698 "interface %d COLORFORMAT error\n",
699 dev->udev->devnum,
700 alts->desc.bInterfaceNumber);
701 return -EINVAL;
702 }
703
704 format->colorspace = uvc_colorspace(buffer[3]);
705
706 buflen -= buffer[0];
707 buffer += buffer[0];
708 }
709
710 return buffer - start;
711 }
712
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59095 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
@ 2019-10-02 12:00 ` kbuild test robot
0 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2019-10-02 12:00 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 11966 bytes --]
Hi Sergey,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[cannot apply to v5.4-rc1 next-20191002]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Sergey-Zakharchenko/media-uvcvideo-Add-a-quirk-to-force-GEO-GC6500-Camera-bits-per-pixel-value/20191002-185359
base: git://linuxtv.org/media_tree.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/media/usb/uvc/uvc_driver.c: In function 'uvc_parse_format.isra.6':
>> drivers/media/usb/uvc/uvc_driver.c:507:18: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized]
n = info->bpp[i] * div;
~~~~~~~~~^~~
vim +/i +507 drivers/media/usb/uvc/uvc_driver.c
436
437 /* ------------------------------------------------------------------------
438 * Descriptors parsing
439 */
440
441 static int uvc_parse_format(struct uvc_device *dev,
442 struct uvc_streaming *streaming, struct uvc_format *format,
443 u32 **intervals, unsigned char *buffer, int buflen)
444 {
445 struct usb_interface *intf = streaming->intf;
446 struct usb_host_interface *alts = intf->cur_altsetting;
447 struct uvc_format_desc *fmtdesc;
448 struct uvc_frame *frame;
449 const struct v4l2_format_info *info;
450 const unsigned char *start = buffer;
451 unsigned int width_multiplier = 1;
452 unsigned int interval;
453 unsigned int i, n;
454 unsigned int div;
455 u8 ftype;
456
457 format->type = buffer[2];
458 format->index = buffer[3];
459
460 switch (buffer[2]) {
461 case UVC_VS_FORMAT_UNCOMPRESSED:
462 case UVC_VS_FORMAT_FRAME_BASED:
463 n = buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED ? 27 : 28;
464 if (buflen < n) {
465 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
466 "interface %d FORMAT error\n",
467 dev->udev->devnum,
468 alts->desc.bInterfaceNumber);
469 return -EINVAL;
470 }
471
472 /* Find the format descriptor from its GUID. */
473 fmtdesc = uvc_format_by_guid(&buffer[5]);
474
475 if (fmtdesc != NULL) {
476 strscpy(format->name, fmtdesc->name,
477 sizeof(format->name));
478 format->fcc = fmtdesc->fcc;
479 } else {
480 uvc_printk(KERN_INFO, "Unknown video format %pUl\n",
481 &buffer[5]);
482 snprintf(format->name, sizeof(format->name), "%pUl\n",
483 &buffer[5]);
484 format->fcc = 0;
485 }
486
487 format->bpp = buffer[21];
488
489 /* Some devices report a format that doesn't match what they
490 * really send.
491 */
492 if (dev->quirks & UVC_QUIRK_FORCE_Y8) {
493 if (format->fcc == V4L2_PIX_FMT_YUYV) {
494 strscpy(format->name, "Greyscale 8-bit (Y8 )",
495 sizeof(format->name));
496 format->fcc = V4L2_PIX_FMT_GREY;
497 format->bpp = 8;
498 width_multiplier = 2;
499 }
500 }
501
502 /* Some devices report bpp that doesn't match the format. */
503 if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
504 info = v4l2_format_info(format->fcc);
505 if (info) {
506 div = info->hdiv * info->vdiv;
> 507 n = info->bpp[i] * div;
508 for (i = 1; i < info->comp_planes; i++)
509 n += info->bpp[i];
510 format->bpp = DIV_ROUND_UP(8 * n, div);
511 }
512 }
513
514 if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
515 ftype = UVC_VS_FRAME_UNCOMPRESSED;
516 } else {
517 ftype = UVC_VS_FRAME_FRAME_BASED;
518 if (buffer[27])
519 format->flags = UVC_FMT_FLAG_COMPRESSED;
520 }
521 break;
522
523 case UVC_VS_FORMAT_MJPEG:
524 if (buflen < 11) {
525 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
526 "interface %d FORMAT error\n",
527 dev->udev->devnum,
528 alts->desc.bInterfaceNumber);
529 return -EINVAL;
530 }
531
532 strscpy(format->name, "MJPEG", sizeof(format->name));
533 format->fcc = V4L2_PIX_FMT_MJPEG;
534 format->flags = UVC_FMT_FLAG_COMPRESSED;
535 format->bpp = 0;
536 ftype = UVC_VS_FRAME_MJPEG;
537 break;
538
539 case UVC_VS_FORMAT_DV:
540 if (buflen < 9) {
541 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
542 "interface %d FORMAT error\n",
543 dev->udev->devnum,
544 alts->desc.bInterfaceNumber);
545 return -EINVAL;
546 }
547
548 switch (buffer[8] & 0x7f) {
549 case 0:
550 strscpy(format->name, "SD-DV", sizeof(format->name));
551 break;
552 case 1:
553 strscpy(format->name, "SDL-DV", sizeof(format->name));
554 break;
555 case 2:
556 strscpy(format->name, "HD-DV", sizeof(format->name));
557 break;
558 default:
559 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
560 "interface %d: unknown DV format %u\n",
561 dev->udev->devnum,
562 alts->desc.bInterfaceNumber, buffer[8]);
563 return -EINVAL;
564 }
565
566 strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
567 sizeof(format->name));
568
569 format->fcc = V4L2_PIX_FMT_DV;
570 format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM;
571 format->bpp = 0;
572 ftype = 0;
573
574 /* Create a dummy frame descriptor. */
575 frame = &format->frame[0];
576 memset(&format->frame[0], 0, sizeof(format->frame[0]));
577 frame->bFrameIntervalType = 1;
578 frame->dwDefaultFrameInterval = 1;
579 frame->dwFrameInterval = *intervals;
580 *(*intervals)++ = 1;
581 format->nframes = 1;
582 break;
583
584 case UVC_VS_FORMAT_MPEG2TS:
585 case UVC_VS_FORMAT_STREAM_BASED:
586 /* Not supported yet. */
587 default:
588 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
589 "interface %d unsupported format %u\n",
590 dev->udev->devnum, alts->desc.bInterfaceNumber,
591 buffer[2]);
592 return -EINVAL;
593 }
594
595 uvc_trace(UVC_TRACE_DESCR, "Found format %s.\n", format->name);
596
597 buflen -= buffer[0];
598 buffer += buffer[0];
599
600 /* Parse the frame descriptors. Only uncompressed, MJPEG and frame
601 * based formats have frame descriptors.
602 */
603 while (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE &&
604 buffer[2] == ftype) {
605 frame = &format->frame[format->nframes];
606 if (ftype != UVC_VS_FRAME_FRAME_BASED)
607 n = buflen > 25 ? buffer[25] : 0;
608 else
609 n = buflen > 21 ? buffer[21] : 0;
610
611 n = n ? n : 3;
612
613 if (buflen < 26 + 4*n) {
614 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
615 "interface %d FRAME error\n", dev->udev->devnum,
616 alts->desc.bInterfaceNumber);
617 return -EINVAL;
618 }
619
620 frame->bFrameIndex = buffer[3];
621 frame->bmCapabilities = buffer[4];
622 frame->wWidth = get_unaligned_le16(&buffer[5])
623 * width_multiplier;
624 frame->wHeight = get_unaligned_le16(&buffer[7]);
625 frame->dwMinBitRate = get_unaligned_le32(&buffer[9]);
626 frame->dwMaxBitRate = get_unaligned_le32(&buffer[13]);
627 if (ftype != UVC_VS_FRAME_FRAME_BASED) {
628 frame->dwMaxVideoFrameBufferSize =
629 get_unaligned_le32(&buffer[17]);
630 frame->dwDefaultFrameInterval =
631 get_unaligned_le32(&buffer[21]);
632 frame->bFrameIntervalType = buffer[25];
633 } else {
634 frame->dwMaxVideoFrameBufferSize = 0;
635 frame->dwDefaultFrameInterval =
636 get_unaligned_le32(&buffer[17]);
637 frame->bFrameIntervalType = buffer[21];
638 }
639 frame->dwFrameInterval = *intervals;
640
641 /* Several UVC chipsets screw up dwMaxVideoFrameBufferSize
642 * completely. Observed behaviours range from setting the
643 * value to 1.1x the actual frame size to hardwiring the
644 * 16 low bits to 0. This results in a higher than necessary
645 * memory usage as well as a wrong image size information. For
646 * uncompressed formats this can be fixed by computing the
647 * value from the frame size.
648 */
649 if (!(format->flags & UVC_FMT_FLAG_COMPRESSED))
650 frame->dwMaxVideoFrameBufferSize = format->bpp
651 * frame->wWidth * frame->wHeight / 8;
652
653 /* Some bogus devices report dwMinFrameInterval equal to
654 * dwMaxFrameInterval and have dwFrameIntervalStep set to
655 * zero. Setting all null intervals to 1 fixes the problem and
656 * some other divisions by zero that could happen.
657 */
658 for (i = 0; i < n; ++i) {
659 interval = get_unaligned_le32(&buffer[26+4*i]);
660 *(*intervals)++ = interval ? interval : 1;
661 }
662
663 /* Make sure that the default frame interval stays between
664 * the boundaries.
665 */
666 n -= frame->bFrameIntervalType ? 1 : 2;
667 frame->dwDefaultFrameInterval =
668 min(frame->dwFrameInterval[n],
669 max(frame->dwFrameInterval[0],
670 frame->dwDefaultFrameInterval));
671
672 if (dev->quirks & UVC_QUIRK_RESTRICT_FRAME_RATE) {
673 frame->bFrameIntervalType = 1;
674 frame->dwFrameInterval[0] =
675 frame->dwDefaultFrameInterval;
676 }
677
678 uvc_trace(UVC_TRACE_DESCR, "- %ux%u (%u.%u fps)\n",
679 frame->wWidth, frame->wHeight,
680 10000000/frame->dwDefaultFrameInterval,
681 (100000000/frame->dwDefaultFrameInterval)%10);
682
683 format->nframes++;
684 buflen -= buffer[0];
685 buffer += buffer[0];
686 }
687
688 if (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE &&
689 buffer[2] == UVC_VS_STILL_IMAGE_FRAME) {
690 buflen -= buffer[0];
691 buffer += buffer[0];
692 }
693
694 if (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE &&
695 buffer[2] == UVC_VS_COLORFORMAT) {
696 if (buflen < 6) {
697 uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
698 "interface %d COLORFORMAT error\n",
699 dev->udev->devnum,
700 alts->desc.bInterfaceNumber);
701 return -EINVAL;
702 }
703
704 format->colorspace = uvc_colorspace(buffer[3]);
705
706 buflen -= buffer[0];
707 buffer += buffer[0];
708 }
709
710 return buffer - start;
711 }
712
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 59095 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 12:00 ` kbuild test robot
(?)
@ 2019-10-02 13:01 ` Sergey Zakharchenko
2019-10-02 14:08 ` Laurent Pinchart
-1 siblings, 1 reply; 12+ messages in thread
From: Sergey Zakharchenko @ 2019-10-02 13:01 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Mauro Carvalho Chehab, Martin Bodo, Logan,
Peter, Auke Kok, Sergey Zakharchenko, Sergey Zakharchenko
This device does not function correctly in raw mode in kernel
versions validating buffer sizes in bulk mode. It erroneously
announces 16 bits per pixel instead of 12 for NV12 format, so it
needs this quirk to fix computed frame size and avoid legitimate
frames getting discarded.
Signed-off-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com>
---
drivers/media/usb/uvc/uvc_driver.c | 27 +++++++++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 66ee168ddc7e..23f62456946d 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -446,10 +446,12 @@ static int uvc_parse_format(struct uvc_device *dev,
struct usb_host_interface *alts = intf->cur_altsetting;
struct uvc_format_desc *fmtdesc;
struct uvc_frame *frame;
+ const struct v4l2_format_info *info;
const unsigned char *start = buffer;
unsigned int width_multiplier = 1;
unsigned int interval;
unsigned int i, n;
+ unsigned int div;
u8 ftype;
format->type = buffer[2];
@@ -497,6 +499,18 @@ static int uvc_parse_format(struct uvc_device *dev,
}
}
+ /* Some devices report bpp that doesn't match the format. */
+ if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
+ info = v4l2_format_info(format->fcc);
+ if (info) {
+ div = info->hdiv * info->vdiv;
+ n = info->bpp[0] * div;
+ for (i = 1; i < info->comp_planes; i++)
+ n += info->bpp[i];
+ format->bpp = DIV_ROUND_UP(8 * n, div);
+ }
+ }
+
if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
ftype = UVC_VS_FRAME_UNCOMPRESSED;
} else {
@@ -2384,6 +2398,10 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
.quirks = UVC_QUIRK_FORCE_Y8,
};
+static const struct uvc_device_info uvc_quirk_force_bpp = {
+ .quirks = UVC_QUIRK_FORCE_BPP,
+};
+
#define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks = q}
#define UVC_INFO_META(m) (kernel_ulong_t)&(struct uvc_device_info) \
{.meta_format = m}
@@ -2869,6 +2887,15 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = UVC_INFO_META(V4L2_META_FMT_D4XX) },
+ /* GEO Semiconductor GC6500 */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x29fe,
+ .idProduct = 0x4d53,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_quirk_force_bpp },
/* Generic USB Video Class */
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) },
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) },
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index c7c1baa90dea..24e3d8c647e7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -198,6 +198,7 @@
#define UVC_QUIRK_RESTRICT_FRAME_RATE 0x00000200
#define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
#define UVC_QUIRK_FORCE_Y8 0x00000800
+#define UVC_QUIRK_FORCE_BPP 0x00001000
/* Format flags */
#define UVC_FMT_FLAG_COMPRESSED 0x00000001
base-commit: 20a438d53fd9d12a894161bc56cbeab7a9993c39
prerequisite-patch-id: 521eb9602d395ea667eecc75cd2273b59cd3ed76
--
2.23.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 13:01 ` [PATCH v2] " Sergey Zakharchenko
@ 2019-10-02 14:08 ` Laurent Pinchart
2019-10-02 14:54 ` Sergey Zakharchenko
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2019-10-02 14:08 UTC (permalink / raw)
To: Sergey Zakharchenko
Cc: linux-media, Mauro Carvalho Chehab, Martin Bodo, Logan, Peter,
Auke Kok, Sergey Zakharchenko
Hi Sergey,
Thank you for the patch.
On Wed, Oct 02, 2019 at 01:01:02PM +0000, Sergey Zakharchenko wrote:
> This device does not function correctly in raw mode in kernel
> versions validating buffer sizes in bulk mode. It erroneously
> announces 16 bits per pixel instead of 12 for NV12 format, so it
> needs this quirk to fix computed frame size and avoid legitimate
> frames getting discarded.
>
> Signed-off-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 27 +++++++++++++++++++++++++++
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 66ee168ddc7e..23f62456946d 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -446,10 +446,12 @@ static int uvc_parse_format(struct uvc_device *dev,
> struct usb_host_interface *alts = intf->cur_altsetting;
> struct uvc_format_desc *fmtdesc;
> struct uvc_frame *frame;
> + const struct v4l2_format_info *info;
> const unsigned char *start = buffer;
> unsigned int width_multiplier = 1;
> unsigned int interval;
> unsigned int i, n;
> + unsigned int div;
> u8 ftype;
>
> format->type = buffer[2];
> @@ -497,6 +499,18 @@ static int uvc_parse_format(struct uvc_device *dev,
> }
> }
>
> + /* Some devices report bpp that doesn't match the format. */
> + if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
> + info = v4l2_format_info(format->fcc);
> + if (info) {
> + div = info->hdiv * info->vdiv;
> + n = info->bpp[0] * div;
> + for (i = 1; i < info->comp_planes; i++)
> + n += info->bpp[i];
> + format->bpp = DIV_ROUND_UP(8 * n, div);
> + }
> + }
Do you think it would make sense to do this by default, without
requiring a quirk ? Or are there cases where this calculation would lead
to incorrect results while the bpp reported by the camera would be right
?
> +
> if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
> ftype = UVC_VS_FRAME_UNCOMPRESSED;
> } else {
> @@ -2384,6 +2398,10 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
> .quirks = UVC_QUIRK_FORCE_Y8,
> };
>
> +static const struct uvc_device_info uvc_quirk_force_bpp = {
> + .quirks = UVC_QUIRK_FORCE_BPP,
> +};
> +
> #define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks = q}
> #define UVC_INFO_META(m) (kernel_ulong_t)&(struct uvc_device_info) \
> {.meta_format = m}
> @@ -2869,6 +2887,15 @@ static const struct usb_device_id uvc_ids[] = {
> .bInterfaceSubClass = 1,
> .bInterfaceProtocol = 0,
> .driver_info = UVC_INFO_META(V4L2_META_FMT_D4XX) },
> + /* GEO Semiconductor GC6500 */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x29fe,
> + .idProduct = 0x4d53,
Could you please keep the entries sorted by idVendor/idProduct ?
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = (kernel_ulong_t)&uvc_quirk_force_bpp },
As this is the only device using this quirk, you can drop
uvc_quirk_force_bpp and use
.driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) },
> /* Generic USB Video Class */
> { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) },
> { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) },
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index c7c1baa90dea..24e3d8c647e7 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -198,6 +198,7 @@
> #define UVC_QUIRK_RESTRICT_FRAME_RATE 0x00000200
> #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> #define UVC_QUIRK_FORCE_Y8 0x00000800
> +#define UVC_QUIRK_FORCE_BPP 0x00001000
>
> /* Format flags */
> #define UVC_FMT_FLAG_COMPRESSED 0x00000001
>
> base-commit: 20a438d53fd9d12a894161bc56cbeab7a9993c39
> prerequisite-patch-id: 521eb9602d395ea667eecc75cd2273b59cd3ed76
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 14:08 ` Laurent Pinchart
@ 2019-10-02 14:54 ` Sergey Zakharchenko
2019-10-02 17:15 ` Sergey Zakharchenko
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Zakharchenko @ 2019-10-02 14:54 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sergey Zakharchenko, linux-media, Mauro Carvalho Chehab,
Martin Bodo, Logan, Peter, Auke Kok
Hi Laurent,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Do you think it would make sense to do this by default, without
> requiring a quirk ? Or are there cases where this calculation would lead
> to incorrect results while the bpp reported by the camera would be right
> ?
I don't nearly have the experience with a broad range of webcams
required to answer this question. At the very least that would tax
well-behaved cameras with a tiny but unnecessary bit of initial
computations.
The loop is a simplified version of the v4l2_fill_pixfmt() loop. The
calculation might need some checking, and might be invalid, in case
block_w/block_h format fields are significant (not 0 and not 1),
because then effective bits-per-pixel would seemingly be fractional,
and depend on the image dimensions if they weren't aligned; however I
see no formats using the block_w/block_h fields defined so far.
AFAICT the division should need no rounding for the raw formats
currently listed; just being cautious there.
> Could you please keep the entries sorted by idVendor/idProduct ?
> As this is the only device using this quirk, you can drop
> uvc_quirk_force_bpp and use
>
> .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) },
All makes sense and will be adjusted, thanks for the review!
Best regards,
--
Sergey Zakharchenko
Digital Loggers, Inc.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 14:54 ` Sergey Zakharchenko
@ 2019-10-02 17:15 ` Sergey Zakharchenko
2019-10-02 19:08 ` Laurent Pinchart
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Zakharchenko @ 2019-10-02 17:15 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sergey Zakharchenko, linux-media, Mauro Carvalho Chehab,
Martin Bodo, Logan, Peter, Auke Kok
Sergey Zakharchenko <doublef.mobile@gmail.com>:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> > Do you think it would make sense to do this by default, without
> > requiring a quirk ? Or are there cases where this calculation would lead
> > to incorrect results while the bpp reported by the camera would be right
> > ?
>
> The loop is a simplified version of the v4l2_fill_pixfmt() loop. The
> calculation might need some checking, and might be invalid, in case
> block_w/block_h format fields are significant (not 0 and not 1),
> because then effective bits-per-pixel would seemingly be fractional,
> and depend on the image dimensions if they weren't aligned; however I
> see no formats using the block_w/block_h fields defined so far.
It's likely possible to directly replace the bpp-using computation in
https://github.com/torvalds/linux/blob/2874c5fd284268364ece81a7bd936f3c8168e567/drivers/media/usb/uvc/uvc_driver.c#L636
with a call to v4l2_fill_pixfmt() and the sizeimage it returns.
However, bpp is used elsewhere, and it's hard to tell what it should
be taken to be to in the hypothetical exotic cases I'm considering, so
I'm reluctant to go that route.
I'm going to send v3 in an hour unless there are other suggestions.
Best regards,
--
Sergey Zakharchenko
Digital Loggers, Inc.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 17:15 ` Sergey Zakharchenko
@ 2019-10-02 19:08 ` Laurent Pinchart
2019-10-02 19:29 ` Sergey Zakharchenko
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2019-10-02 19:08 UTC (permalink / raw)
To: Sergey Zakharchenko
Cc: Sergey Zakharchenko, linux-media, Mauro Carvalho Chehab,
Martin Bodo, Logan, Peter, Auke Kok
Hi Sergey,
On Wed, Oct 02, 2019 at 09:15:45PM +0400, Sergey Zakharchenko wrote:
> Sergey Zakharchenko <doublef.mobile@gmail.com>:
> > Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> > > Do you think it would make sense to do this by default, without
> > > requiring a quirk ? Or are there cases where this calculation would lead
> > > to incorrect results while the bpp reported by the camera would be right
> > > ?
> >
> > The loop is a simplified version of the v4l2_fill_pixfmt() loop. The
> > calculation might need some checking, and might be invalid, in case
> > block_w/block_h format fields are significant (not 0 and not 1),
> > because then effective bits-per-pixel would seemingly be fractional,
> > and depend on the image dimensions if they weren't aligned; however I
> > see no formats using the block_w/block_h fields defined so far.
>
> It's likely possible to directly replace the bpp-using computation in
> https://github.com/torvalds/linux/blob/2874c5fd284268364ece81a7bd936f3c8168e567/drivers/media/usb/uvc/uvc_driver.c#L636
> with a call to v4l2_fill_pixfmt() and the sizeimage it returns.
> However, bpp is used elsewhere, and it's hard to tell what it should
> be taken to be to in the hypothetical exotic cases I'm considering, so
> I'm reluctant to go that route.
Would it make sense to split the calculation from v4l2_fill_pixfmt() to
a helper function that the UVC driver could call ?
> I'm going to send v3 in an hour unless there are other suggestions.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 19:08 ` Laurent Pinchart
@ 2019-10-02 19:29 ` Sergey Zakharchenko
2019-10-03 9:31 ` [PATCH v3] " Sergey Zakharchenko
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Zakharchenko @ 2019-10-02 19:29 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sergey Zakharchenko, linux-media, Mauro Carvalho Chehab,
Martin Bodo, Logan, Peter, Auke Kok
Laurent,
> Would it make sense to split the calculation from v4l2_fill_pixfmt() to
> a helper function that the UVC driver could call ?
It would of course be possible, and would benefit v4l2-common.c as it
would be common between v4l2_fill_pixfmt() and v4l2_fill_pixfmt_mp(),
but as I noted before that approach would leave the bpp struct member,
used elsewhere, possibly incorrect. I'm also not sure where
multi-planar formats fit in (probably not related to the problem at
hand).
> > I'm going to send v3 in an hour unless there are other suggestions.
Till tomorrow, then...
Best regards,
--
Sergey Zakharchenko
Digital Loggers, Inc.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-02 19:29 ` Sergey Zakharchenko
@ 2019-10-03 9:31 ` Sergey Zakharchenko
2019-12-18 18:00 ` Laurent Pinchart
0 siblings, 1 reply; 12+ messages in thread
From: Sergey Zakharchenko @ 2019-10-03 9:31 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Mauro Carvalho Chehab, Martin Bodo, Logan,
Peter, Auke Kok, Sergey Zakharchenko, Sergey Zakharchenko
This device does not function correctly in raw mode in kernel
versions validating buffer sizes in bulk mode. It erroneously
announces 16 bits per pixel instead of 12 for NV12 format, so it
needs this quirk to fix computed frame size and avoid legitimate
frames getting discarded.
Signed-off-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com>
---
drivers/media/usb/uvc/uvc_driver.c | 23 +++++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 66ee168ddc7e..d63db65ef8b2 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -446,10 +446,12 @@ static int uvc_parse_format(struct uvc_device *dev,
struct usb_host_interface *alts = intf->cur_altsetting;
struct uvc_format_desc *fmtdesc;
struct uvc_frame *frame;
+ const struct v4l2_format_info *info;
const unsigned char *start = buffer;
unsigned int width_multiplier = 1;
unsigned int interval;
unsigned int i, n;
+ unsigned int div;
u8 ftype;
format->type = buffer[2];
@@ -497,6 +499,18 @@ static int uvc_parse_format(struct uvc_device *dev,
}
}
+ /* Some devices report bpp that doesn't match the format. */
+ if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
+ info = v4l2_format_info(format->fcc);
+ if (info) {
+ div = info->hdiv * info->vdiv;
+ n = info->bpp[0] * div;
+ for (i = 1; i < info->comp_planes; i++)
+ n += info->bpp[i];
+ format->bpp = DIV_ROUND_UP(8 * n, div);
+ }
+ }
+
if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
ftype = UVC_VS_FRAME_UNCOMPRESSED;
} else {
@@ -2860,6 +2874,15 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = (kernel_ulong_t)&uvc_quirk_force_y8 },
+ /* GEO Semiconductor GC6500 */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x29fe,
+ .idProduct = 0x4d53,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) },
/* Intel RealSense D4M */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index c7c1baa90dea..24e3d8c647e7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -198,6 +198,7 @@
#define UVC_QUIRK_RESTRICT_FRAME_RATE 0x00000200
#define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
#define UVC_QUIRK_FORCE_Y8 0x00000800
+#define UVC_QUIRK_FORCE_BPP 0x00001000
/* Format flags */
#define UVC_FMT_FLAG_COMPRESSED 0x00000001
base-commit: 20a438d53fd9d12a894161bc56cbeab7a9993c39
prerequisite-patch-id: 521eb9602d395ea667eecc75cd2273b59cd3ed76
--
2.23.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-10-03 9:31 ` [PATCH v3] " Sergey Zakharchenko
@ 2019-12-18 18:00 ` Laurent Pinchart
2019-12-18 18:46 ` Sergey Zakharchenko
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2019-12-18 18:00 UTC (permalink / raw)
To: Sergey Zakharchenko
Cc: linux-media, Mauro Carvalho Chehab, Martin Bodo, Logan, Peter,
Auke Kok, Sergey Zakharchenko
Hi Sergey,
I'm sorry for the late reply, this patch fell through the cracks :-S
On Thu, Oct 03, 2019 at 09:31:23AM +0000, Sergey Zakharchenko wrote:
> This device does not function correctly in raw mode in kernel
> versions validating buffer sizes in bulk mode. It erroneously
> announces 16 bits per pixel instead of 12 for NV12 format, so it
> needs this quirk to fix computed frame size and avoid legitimate
> frames getting discarded.
>
> Signed-off-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 23 +++++++++++++++++++++++
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 66ee168ddc7e..d63db65ef8b2 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -446,10 +446,12 @@ static int uvc_parse_format(struct uvc_device *dev,
> struct usb_host_interface *alts = intf->cur_altsetting;
> struct uvc_format_desc *fmtdesc;
> struct uvc_frame *frame;
> + const struct v4l2_format_info *info;
> const unsigned char *start = buffer;
> unsigned int width_multiplier = 1;
> unsigned int interval;
> unsigned int i, n;
> + unsigned int div;
> u8 ftype;
>
> format->type = buffer[2];
> @@ -497,6 +499,18 @@ static int uvc_parse_format(struct uvc_device *dev,
> }
> }
>
> + /* Some devices report bpp that doesn't match the format. */
> + if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
Both info and div can be declared here. The patch otherwise looks good
to me, so I'll make this modification when applying it, unless you would
prefer resending it.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
I however had a closer look at how bpp is used, and I think it would
make sense to actually drop usage of this field. There's a usage in
uvc_video.c that is easy to replace (I will send a patch and CC you),
and I think the other two usages in uvc_driver.c and uvc_v4l2.c to
compute the image size and line size respectively should eventually be
replaced by calculation based on the format info.
> + info = v4l2_format_info(format->fcc);
> + if (info) {
> + div = info->hdiv * info->vdiv;
> + n = info->bpp[0] * div;
> + for (i = 1; i < info->comp_planes; i++)
> + n += info->bpp[i];
> + format->bpp = DIV_ROUND_UP(8 * n, div);
> + }
> + }
> +
> if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
> ftype = UVC_VS_FRAME_UNCOMPRESSED;
> } else {
> @@ -2860,6 +2874,15 @@ static const struct usb_device_id uvc_ids[] = {
> .bInterfaceSubClass = 1,
> .bInterfaceProtocol = 0,
> .driver_info = (kernel_ulong_t)&uvc_quirk_force_y8 },
> + /* GEO Semiconductor GC6500 */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x29fe,
> + .idProduct = 0x4d53,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) },
> /* Intel RealSense D4M */
> { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> | USB_DEVICE_ID_MATCH_INT_INFO,
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index c7c1baa90dea..24e3d8c647e7 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -198,6 +198,7 @@
> #define UVC_QUIRK_RESTRICT_FRAME_RATE 0x00000200
> #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> #define UVC_QUIRK_FORCE_Y8 0x00000800
> +#define UVC_QUIRK_FORCE_BPP 0x00001000
>
> /* Format flags */
> #define UVC_FMT_FLAG_COMPRESSED 0x00000001
>
> base-commit: 20a438d53fd9d12a894161bc56cbeab7a9993c39
> prerequisite-patch-id: 521eb9602d395ea667eecc75cd2273b59cd3ed76
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
2019-12-18 18:00 ` Laurent Pinchart
@ 2019-12-18 18:46 ` Sergey Zakharchenko
0 siblings, 0 replies; 12+ messages in thread
From: Sergey Zakharchenko @ 2019-12-18 18:46 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Mauro Carvalho Chehab, Martin Bodo, Logan, Peter,
Auke Kok, Sergey Zakharchenko
Hello Laurent,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> I'm sorry for the late reply, this patch fell through the cracks :-S
It's OK, thanks for getting back to it!
> > + /* Some devices report bpp that doesn't match the format. */
> > + if (dev->quirks & UVC_QUIRK_FORCE_BPP) {
>
> Both info and div can be declared here.
Right; however I was just following what seemed to be the existing
practice of declaring all variables at the start of a function (which
I'm not fond of myself; e.g. fmtdesc is only used in a single switch
branch). Now after looking at the code more, there are precedents for
declaring closer to point of use in e.g. uvc_parse_control and
elsewhere, so this is probably OK.
> The patch otherwise looks good
> to me, so I'll make this modification when applying it, unless you would
> prefer resending it.
Sure, go ahead, and thanks!
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I however had a closer look at how bpp is used, and I think it would
> make sense to actually drop usage of this field.
This is most probably possible (the only cases I could think of in
which it couldn't be calculated, it would have to be fractional or no
meaningful value could be assigned to it) but out of scope of this
patch.
Best regards,
--
Sergey Zakharchenko
Digital Loggers, Inc.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-12-18 18:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 8:47 [PATCH] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value Sergey Zakharchenko
2019-10-02 12:00 ` kbuild test robot
2019-10-02 12:00 ` kbuild test robot
2019-10-02 13:01 ` [PATCH v2] " Sergey Zakharchenko
2019-10-02 14:08 ` Laurent Pinchart
2019-10-02 14:54 ` Sergey Zakharchenko
2019-10-02 17:15 ` Sergey Zakharchenko
2019-10-02 19:08 ` Laurent Pinchart
2019-10-02 19:29 ` Sergey Zakharchenko
2019-10-03 9:31 ` [PATCH v3] " Sergey Zakharchenko
2019-12-18 18:00 ` Laurent Pinchart
2019-12-18 18:46 ` Sergey Zakharchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.