On 17/07/15(Fri) 17:53, Ludovic Coues wrote:
> Following yesterday feedback, I wrote a patch merging
> usb_video_header_desc and usb_video_header_desc_all in uvideo.c .
> Current kernel compile fine with it on amd64 and video display image.
>
> At the moment, I can't test the patch on other platform. So I'm looking
> for feedback, on the pattern used and if it change anything on
> non-amd64. If those are positive, there is a couple of structure I would
> alter in the same way.
As I got no feedback on this one, I've gone ahead and changed a few more
structure and fixed code using these in uvideo.c .
usb_video_header_desc and usb_video_input_header_desc where separated
into two structure for historical limitation.
usb_video_input_terminal_desc has been turned into a fixed length
structure by following the 1.5 specification. Last field isn't a pointer
anymore but an array of length 3. I have no reason to believe the 1.5
spec break backward compatibility with 1.1 so I assume this change won't
break existing video usb device. Same logic is applied to
usb_video_vc_processing_desc. As a bonus, this one get direct access to the 2
last field.
Lastly, usb_video_frame_desc got it last field. Prior to this patch,
data where accessed by pointer arithmetic. Now, data are in their own
field. The trick is when bFrameIntervalType is set to 0, it's followed
by 3 values. When it's set to non-zero value, it's the number of
following value. To represent that, I've made the last value a unamed
union with 2 named struct. This work well on amd64 but I don't know if
all version of gcc will accept such structure.
With this patch and the one adding missing descriptor, uvideo should be
up to date with the 1.5 version of the usb video specification;
Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
retrieving revision 1.181
diff -u -p -r1.181 uvideo.c
--- sys/dev/usb/uvideo.c 9 Jul 2015 14:58:32 -0000 1.181
+++ sys/dev/usb/uvideo.c 23 Jul 2015 14:31:18 -0000
@@ -84,8 +84,8 @@ struct uvideo_softc {
int sc_nframes;
struct usb_video_probe_commit sc_desc_probe;
- struct usb_video_header_desc_all sc_desc_vc_header;
- struct usb_video_input_header_desc_all sc_desc_vs_input_header;
+ struct usb_video_header_desc *sc_desc_vc_header;
+ struct usb_video_input_header_desc *sc_desc_vs_input_header;
#define UVIDEO_MAX_PU 8
int sc_desc_vc_pu_num;
@@ -668,8 +668,7 @@ uvideo_vc_parse_desc(struct uvideo_softc
break;
case UDESCSUB_VC_PROCESSING_UNIT:
/* XXX do correct length calculation */
- if (desc->bLength <
- sizeof(struct usb_video_frame_desc)) {
+ if (desc->bLength < UVIDEO_FRAME_DESC_LENGTH) {
(void)uvideo_vc_parse_desc_pu(sc, desc);
}
break;
@@ -694,16 +693,15 @@ uvideo_vc_parse_desc_header(struct uvide
{
struct usb_video_header_desc *d;
- d = (struct usb_video_header_desc *)(uint8_t *)desc;
+ d = (struct usb_video_header_desc *)desc;
if (d->bInCollection == 0) {
printf("%s: no VS interface found!\n",
DEVNAME(sc));
return (USBD_INVAL);
}
-
- sc->sc_desc_vc_header.fix = d;
- sc->sc_desc_vc_header.baInterfaceNr = (uByte *)(d + 1);
+
+ sc->sc_desc_vc_header = d;
return (USBD_NORMAL_COMPLETION);
}
@@ -838,7 +836,7 @@ uvideo_vs_parse_desc(struct uvideo_softc
DPRINTF(1, "%s: number of total interfaces=%d\n",
DEVNAME(sc), sc->sc_nifaces);
DPRINTF(1, "%s: number of VS interfaces=%d\n",
- DEVNAME(sc), sc->sc_desc_vc_header.fix->bInCollection);
+ DEVNAME(sc), sc->sc_desc_vc_header->bInCollection);
usbd_desc_iter_init(sc->sc_udev, &iter);
desc = usbd_desc_iter_next(&iter);
@@ -874,8 +872,8 @@ uvideo_vs_parse_desc(struct uvideo_softc
return (error);
/* parse interface collection */
- for (i = 0; i < sc->sc_desc_vc_header.fix->bInCollection; i++) {
- iface = sc->sc_desc_vc_header.baInterfaceNr[i];
+ for (i = 0; i < sc->sc_desc_vc_header->bInCollection; i++) {
+ iface = sc->sc_desc_vc_header->baInterfaceNr[i];
id = usbd_get_interface_descriptor(&sc->sc_udev->ifaces[iface]);
if (id == NULL) {
@@ -916,8 +914,7 @@ uvideo_vs_parse_desc_input_header(struct
return (USBD_INVAL);
}
- sc->sc_desc_vs_input_header.fix = d;
- sc->sc_desc_vs_input_header.bmaControls = (uByte *)(d + 1);
+ sc->sc_desc_vs_input_header = d;
return (USBD_NORMAL_COMPLETION);
}
@@ -1075,7 +1072,8 @@ uvideo_vs_parse_desc_frame(struct uvideo
if (desc->bDescriptorType == UDESC_CS_INTERFACE &&
desc->bLength > sizeof(struct usb_video_frame_desc) &&
(desc->bDescriptorSubtype == UDESCSUB_VS_FRAME_MJPEG ||
- desc->bDescriptorSubtype ==
UDESCSUB_VS_FRAME_UNCOMPRESSED)) {
+ desc->bDescriptorSubtype == UDESCSUB_VS_FRAME_UNCOMPRESSED)
+ ) {
error = uvideo_vs_parse_desc_frame_sub(sc, desc);
if (error != USBD_NORMAL_COMPLETION)
return (error);
@@ -1090,8 +1088,8 @@ usbd_status
uvideo_vs_parse_desc_frame_sub(struct uvideo_softc *sc,
const usb_descriptor_t *desc)
{
- struct usb_video_frame_desc *fd =
- (struct usb_video_frame_desc *)(uint8_t *)desc;
+ /* Descriptor is variable sized */
+ struct usb_video_frame_desc *fd = (void *)desc;
int fmtidx, frame_num;
uint32_t fbuf_size;
@@ -1500,12 +1498,12 @@ uvideo_vs_negotiation(struct uvideo_soft
* Some UVC 1.00 devices return dwMaxVideoFrameSize = 0.
* If so, fix it by format/frame descriptors.
*/
- hd = sc->sc_desc_vc_header.fix;
+ hd = sc->sc_desc_vc_header;
if (UGETDW(pc->dwMaxVideoFrameSize) == 0 &&
UGETW(hd->bcdUVC) < 0x0110 ) {
DPRINTF(1, "%s: dwMaxVideoFrameSize == 0, fixed\n",
DEVNAME(sc));
- USETDW(pc->dwMaxVideoFrameSize,
+ USETDW(pc->dwMaxVideoFrameSize,
UGETDW(frame->dwMaxVideoFrameBufferSize));
}
}
@@ -2629,10 +2627,10 @@ void
uvideo_dump_desc_frame(struct uvideo_softc *sc, const usb_descriptor_t *desc)
{
struct usb_video_frame_desc *d;
- uint8_t *p;
- int length, i;
+ int i;
- d = (struct usb_video_frame_desc *)(uint8_t *)desc;
+ /* Descriptor is variable sized */
+ d = (void*) desc;
printf("bLength=%d\n", d->bLength);
printf("bDescriptorType=0x%02x\n", d->bDescriptorType);
@@ -2649,33 +2647,23 @@ uvideo_dump_desc_frame(struct uvideo_sof
UGETDW(d->dwDefaultFrameInterval));
printf("bFrameIntervalType=0x%02x\n", d->bFrameIntervalType);
- p = (uint8_t *)d;
- p += sizeof(struct usb_video_frame_desc);
-
- if (!d->bFrameIntervalType) {
+ if (d->bFrameIntervalType == 0) {
/* continuous */
- if (d->bLength < (sizeof(struct usb_video_frame_desc) +
- sizeof(uDWord) * 3)) {
- printf("invalid frame descriptor length\n");
- } else {
- printf("dwMinFrameInterval = %d\n", UGETDW(p));
- p += sizeof(uDWord);
- printf("dwMaxFrameInterval = %d\n", UGETDW(p));
- p += sizeof(uDWord);
- printf("dwFrameIntervalStep = %d\n", UGETDW(p));
- p += sizeof(uDWord);
- }
+ printf("dwMinFrameInterval = %d\n",
+ UGETDW(d->continuous.dwMinFrameInterval));
+ printf("dwMaxFrameInterval = %d\n",
+ UGETDW(d->continuous.dwMaxFrameInterval));
+ printf("dwFrameIntervalStep = %d\n",
+ UGETDW(d->continuous.dwFrameIntervalStep));
} else {
/* discrete */
- length = d->bLength - sizeof(struct usb_video_frame_desc);
for (i = 0; i < d->bFrameIntervalType; i++) {
- if (length <= 0) {
+ if (d->bLength < UVIDEO_FRAME_DESC_LENGTH + i) {
printf("frame descriptor ended early\n");
break;
}
- printf("dwFrameInterval = %d\n", UGETDW(p));
- p += sizeof(uDWord);
- length -= sizeof(uDWord);
+ printf("dwFrameInterval = %d\n",
+ UGETDW(d->discrete.dwFrameInterval[i]));
}
}
}
@@ -2708,8 +2696,7 @@ uvideo_dump_desc_processing(struct uvide
{
struct usb_video_vc_processing_desc *d;
- /* PU descriptor is variable sized */
- d = (void *)desc;
+ d = (struct usb_video_vc_processing_desc *)(uint8_t *)desc;
printf("bLength=%d\n", d->bLength);
printf("bDescriptorType=0x%02x\n", d->bDescriptorType);
@@ -2720,8 +2707,8 @@ uvideo_dump_desc_processing(struct uvide
printf("bControlSize=%d\n", d->bControlSize);
printf("bmControls=0x");
uvideo_hexdump(d->bmControls, d->bControlSize, 1);
- printf("iProcessing=0x%02x\n", d->bmControls[d->bControlSize]);
- printf("bmVideoStandards=0x%02x\n", d->bmControls[d->bControlSize + 1]);
+ printf("iProcessing=0x%02x\n", d->iProcessing);
+ printf("bmVideoStandards=0x%02x\n", d->bmVideoStandards);
}
void
@@ -2924,7 +2911,6 @@ uvideo_enum_fivals(void *v, struct v4l2_
int idx;
struct uvideo_format_group *fmtgrp = NULL;
struct usb_video_frame_desc *frame = NULL;
- uint8_t *p;
for (idx = 0; idx < sc->sc_fmtgrp_num; idx++) {
if (sc->sc_fmtgrp[idx].pixelformat == fivals->pixel_format) {
@@ -2945,33 +2931,29 @@ uvideo_enum_fivals(void *v, struct v4l2_
if (frame == NULL)
return (EINVAL);
- /* byte-wise pointer to start of frame intervals */
- p = (uint8_t *)frame;
- p += sizeof(struct usb_video_frame_desc);
-
if (frame->bFrameIntervalType == 0) {
if (fivals->index != 0)
return (EINVAL);
fivals->type = V4L2_FRMIVAL_TYPE_STEPWISE;
- fivals->stepwise.min.numerator = UGETDW(p);
+ fivals->stepwise.min.numerator =
+ UGETDW(frame->continuous.dwMinFrameInterval);
fivals->stepwise.min.denominator = 10000000;
- p += sizeof(uDWord);
- fivals->stepwise.max.numerator = UGETDW(p);
+ fivals->stepwise.max.numerator =
+ UGETDW(frame->continuous.dwMaxFrameInterval);
fivals->stepwise.max.denominator = 10000000;
- p += sizeof(uDWord);
- fivals->stepwise.step.numerator = UGETDW(p);
+ fivals->stepwise.step.numerator =
+ UGETDW(frame->continuous.dwFrameIntervalStep);
fivals->stepwise.step.denominator = 10000000;
- p += sizeof(uDWord);
} else {
if (fivals->index >= frame->bFrameIntervalType)
return (EINVAL);
- p += sizeof(uDWord) * fivals->index;
- if (p > frame->bLength + (uint8_t *)frame) {
+ if (fivals->index + UVIDEO_FRAME_DESC_LENGTH > frame->bLength){
printf("%s: frame desc too short?\n", __func__);
return (EINVAL);
}
fivals->type = V4L2_FRMIVAL_TYPE_DISCRETE;
- fivals->discrete.numerator = UGETDW(p);
+ fivals->discrete.numerator =
+ UGETDW(frame->discrete.dwFrameInterval[fivals->index]);
fivals->discrete.denominator = 10000000;
}
Index: sys/dev/usb/uvideo.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/uvideo.h,v
retrieving revision 1.57
diff -u -p -r1.57 uvideo.h
--- sys/dev/usb/uvideo.h 9 Jul 2015 14:58:32 -0000 1.57
+++ sys/dev/usb/uvideo.h 23 Jul 2015 14:31:18 -0000
@@ -162,13 +162,9 @@ struct usb_video_header_desc {
uWord wTotalLength;
uDWord dwClockFrequency;
uByte bInCollection;
+ uByte baInterfaceNr[1];
} __packed;
-struct usb_video_header_desc_all {
- struct usb_video_header_desc *fix;
- uByte *baInterfaceNr;
-};
-
/* Table 3-4: Input Terminal Descriptor */
struct usb_video_input_terminal_desc {
uByte bLength;
@@ -205,7 +201,7 @@ struct usb_video_camera_terminal_desc {
uWord wObjectiveFocalLengthMax;
uWord wOcularFocalLength;
uByte bControlSize;
- uByte *bmControls;
+ uByte bmControls[3];
} __packed;
/* Table 3-8: VC Processing Unit Descriptor */
@@ -217,9 +213,9 @@ struct usb_video_vc_processing_desc {
uByte bSourceID;
uWord wMaxMultiplier;
uByte bControlSize;
- uByte bmControls[255]; /* [bControlSize] */
- /* uByte iProcessing; */
- /* uByte bmVideoStandards; */
+ uByte bmControls[3];
+ uByte iProcessing;
+ uByte bmVideoStandards;
} __packed;
/* Table 3-9: VC Extension Unit Descriptor */
@@ -255,13 +251,9 @@ struct usb_video_input_header_desc {
uByte bTriggerSupport;
uByte bTriggerUsage;
uByte bControlSize;
+ uByte bmaControls[1];
} __packed;
-struct usb_video_input_header_desc_all {
- struct usb_video_input_header_desc *fix;
- uByte *bmaControls;
-};
-
/* Table 3-18: Color Matching Descriptor */
struct usb_video_color_matching_descr {
uByte bLength;
@@ -357,7 +349,17 @@ struct usb_video_frame_desc {
uDWord dwMaxVideoFrameBufferSize;
uDWord dwDefaultFrameInterval;
uByte bFrameIntervalType;
- /* uDWord ivals[]; frame intervals, length varies */
+#define UVIDEO_FRAME_DESC_LENGTH 26 /* That's the size up to this point */
+ union {
+ struct {
+ uDWord dwMinFrameInterval;
+ uDWord dwMaxFrameInterval;
+ uDWord dwFrameIntervalStep;
+ } continuous;
+ struct {
+ uDWord dwFrameInterval[1];
+ } discrete;
+ };
} __packed;
/*
