Re: [Linux-uvc-devel] uvc header question
Hi Alexey, On Wednesday 07 September 2011 10:46:41 Alexey Fisher wrote: On 07.09.2011 10:05, Laurent Pinchart wrote: On Wednesday 07 September 2011 08:14:45 Alexey Fisher wrote: Am 06.09.2011 17:24, schrieb Laurent Pinchart: On Monday 05 September 2011 17:48:42 Alexey Fisher wrote: Am 31.08.2011 00:32, schrieb Laurent Pinchart: On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote: Hi Laurent, are there any reason why uvc_video_decode_start do not do precise header size checks? Are there many cameras with broken header size too? How precise do you mean ? The driver currently doesn't use much of the header, so it just makes sure that the header size is smaller than or equal to the packet size, and that it's at least 2 bytes long. I send you patch on what i work now to catch streams with fragmented packets.. what do you think about it? Will you apply some thing like this? I'm not sure about that. Webcams that would require something like that are so broken that I'm tempted to consider them as not UVC-compliant. They should be returned to vendors with a loud complaint. Your patch might help, but the sad story is that it can't completely fix the streams. There's always a chance that fragmented packets that contains no header will start with data that looks like a header. You won't be able to find a buller-proof solution. You are right, the idea is not to show definitely broken frames. If there is some thing what we can't filter, is ok. we did our best. I understand. I'm not sure if this should be included in the mainline uvcvideo driver though. It makes the code more complex to support a couple of completely broken devices, and doesn't guarantee that those devices will work correctly. ok, i give up. I'm sorry about that. I definitely appreciate the work you've done on this, but as I said I don't think we should cripple the driver with complex code just to try to support a bit better a webcam that is completely broken anyway. I just thinking about build in uvc compliance tester insight of the module. Some thing what users can use right in shop or at home before 14 day return guarantee. You enables compliance test and it print results in in dmesg. One of test should be header check, error/drop rate, and so on. That's an interesting idea. It should probably come with a userspace stress test software as well. you mean luvcview or some thing other? I'm thinking about a new command line software that would get/set controls at high speed for instance, verify that all controls reported by the device are actually supported, start/stop streaming multiple times, ... Something that would stress the device and try to crash it (which is unfortunately often quite easy :-)). That's nice :) and on kernel should provide file with statistics. Some thing like: /sys/kernel/debug/uvcvideo/stats It should have - packet count - frame count - payloads with error bit - empthy payloads - overruns, and underruns (for row) - jpeg completation (SOI - EOI) I don't think the driver should parse MJPEG data to extract markers. This can be done by the userspace application. Any thing else? The number of failed and total control requests would also be interesting. Statistics about the status endpoint would also be interesting, to verify that the device correctly sends control change events for instance (although those are not properly supported in the driver yet). -- Regards, Laurent Pinchart ___ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
Re: [Linux-uvc-devel] uvc header question
Hi Laurent, Am 06.09.2011 17:24, schrieb Laurent Pinchart: Hi Alexey, On Monday 05 September 2011 17:48:42 Alexey Fisher wrote: Am 31.08.2011 00:32, schrieb Laurent Pinchart: On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote: Hi Laurent, are there any reason why uvc_video_decode_start do not do precise header size checks? Are there many cameras with broken header size too? How precise do you mean ? The driver currently doesn't use much of the header, so it just makes sure that the header size is smaller than or equal to the packet size, and that it's at least 2 bytes long. I send you patch on what i work now to catch streams with fragmented packets.. what do you think about it? Will you apply some thing like this? I'm not sure about that. Webcams that would require something like that are so broken that I'm tempted to consider them as not UVC-compliant. They should be returned to vendors with a loud complaint. Your patch might help, but the sad story is that it can't completely fix the streams. There's always a chance that fragmented packets that contains no header will start with data that looks like a header. You won't be able to find a buller-proof solution. You are right, the idea is not to show definitely broken frames. If there is some thing what we can't filter, is ok. we did our best. I understand. I'm not sure if this should be included in the mainline uvcvideo driver though. It makes the code more complex to support a couple of completely broken devices, and doesn't guarantee that those devices will work correctly. ok, i give up. I just thinking about build in uvc compliance tester insight of the module. Some thing what users can use right in shop or at home before 14 day return guarantee. You enables compliance test and it print results in in dmesg. One of test should be header check, error/drop rate, and so on. That's an interesting idea. It should probably come with a userspace stress test software as well. you mean luvcview or some thing other? ___ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
Re: [Linux-uvc-devel] uvc header question
Hi Alexey, On Wednesday 07 September 2011 08:14:45 Alexey Fisher wrote: Am 06.09.2011 17:24, schrieb Laurent Pinchart: On Monday 05 September 2011 17:48:42 Alexey Fisher wrote: Am 31.08.2011 00:32, schrieb Laurent Pinchart: On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote: Hi Laurent, are there any reason why uvc_video_decode_start do not do precise header size checks? Are there many cameras with broken header size too? How precise do you mean ? The driver currently doesn't use much of the header, so it just makes sure that the header size is smaller than or equal to the packet size, and that it's at least 2 bytes long. I send you patch on what i work now to catch streams with fragmented packets.. what do you think about it? Will you apply some thing like this? I'm not sure about that. Webcams that would require something like that are so broken that I'm tempted to consider them as not UVC-compliant. They should be returned to vendors with a loud complaint. Your patch might help, but the sad story is that it can't completely fix the streams. There's always a chance that fragmented packets that contains no header will start with data that looks like a header. You won't be able to find a buller-proof solution. You are right, the idea is not to show definitely broken frames. If there is some thing what we can't filter, is ok. we did our best. I understand. I'm not sure if this should be included in the mainline uvcvideo driver though. It makes the code more complex to support a couple of completely broken devices, and doesn't guarantee that those devices will work correctly. ok, i give up. I'm sorry about that. I definitely appreciate the work you've done on this, but as I said I don't think we should cripple the driver with complex code just to try to support a bit better a webcam that is completely broken anyway. I just thinking about build in uvc compliance tester insight of the module. Some thing what users can use right in shop or at home before 14 day return guarantee. You enables compliance test and it print results in in dmesg. One of test should be header check, error/drop rate, and so on. That's an interesting idea. It should probably come with a userspace stress test software as well. you mean luvcview or some thing other? I'm thinking about a new command line software that would get/set controls at high speed for instance, verify that all controls reported by the device are actually supported, start/stop streaming multiple times, ... Something that would stress the device and try to crash it (which is unfortunately often quite easy :-)). -- Regards, Laurent Pinchart ___ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
Re: [Linux-uvc-devel] uvc header question
On 07.09.2011 10:05, Laurent Pinchart wrote: Hi Alexey, On Wednesday 07 September 2011 08:14:45 Alexey Fisher wrote: Am 06.09.2011 17:24, schrieb Laurent Pinchart: On Monday 05 September 2011 17:48:42 Alexey Fisher wrote: Am 31.08.2011 00:32, schrieb Laurent Pinchart: On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote: Hi Laurent, are there any reason why uvc_video_decode_start do not do precise header size checks? Are there many cameras with broken header size too? How precise do you mean ? The driver currently doesn't use much of the header, so it just makes sure that the header size is smaller than or equal to the packet size, and that it's at least 2 bytes long. I send you patch on what i work now to catch streams with fragmented packets.. what do you think about it? Will you apply some thing like this? I'm not sure about that. Webcams that would require something like that are so broken that I'm tempted to consider them as not UVC-compliant. They should be returned to vendors with a loud complaint. Your patch might help, but the sad story is that it can't completely fix the streams. There's always a chance that fragmented packets that contains no header will start with data that looks like a header. You won't be able to find a buller-proof solution. You are right, the idea is not to show definitely broken frames. If there is some thing what we can't filter, is ok. we did our best. I understand. I'm not sure if this should be included in the mainline uvcvideo driver though. It makes the code more complex to support a couple of completely broken devices, and doesn't guarantee that those devices will work correctly. ok, i give up. I'm sorry about that. I definitely appreciate the work you've done on this, but as I said I don't think we should cripple the driver with complex code just to try to support a bit better a webcam that is completely broken anyway. I just thinking about build in uvc compliance tester insight of the module. Some thing what users can use right in shop or at home before 14 day return guarantee. You enables compliance test and it print results in in dmesg. One of test should be header check, error/drop rate, and so on. That's an interesting idea. It should probably come with a userspace stress test software as well. you mean luvcview or some thing other? I'm thinking about a new command line software that would get/set controls at high speed for instance, verify that all controls reported by the device are actually supported, start/stop streaming multiple times, ... Something that would stress the device and try to crash it (which is unfortunately often quite easy :-)). That's nice :) and on kernel should provide file with statistics. Some thing like: /sys/kernel/debug/uvcvideo/stats It should have - packet count - frame count - payloads with error bit - empthy payloads - overruns, and underruns (for row) - jpeg completation (SOI - EOI) Any thing else? ___ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
Re: [Linux-uvc-devel] uvc header question
Hi Alexey, On Monday 05 September 2011 17:48:42 Alexey Fisher wrote: Am 31.08.2011 00:32, schrieb Laurent Pinchart: On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote: Hi Laurent, are there any reason why uvc_video_decode_start do not do precise header size checks? Are there many cameras with broken header size too? How precise do you mean ? The driver currently doesn't use much of the header, so it just makes sure that the header size is smaller than or equal to the packet size, and that it's at least 2 bytes long. I send you patch on what i work now to catch streams with fragmented packets.. what do you think about it? Will you apply some thing like this? I'm not sure about that. Webcams that would require something like that are so broken that I'm tempted to consider them as not UVC-compliant. They should be returned to vendors with a loud complaint. Your patch might help, but the sad story is that it can't completely fix the streams. There's always a chance that fragmented packets that contains no header will start with data that looks like a header. You won't be able to find a buller-proof solution. You are right, the idea is not to show definitely broken frames. If there is some thing what we can't filter, is ok. we did our best. I understand. I'm not sure if this should be included in the mainline uvcvideo driver though. It makes the code more complex to support a couple of completely broken devices, and doesn't guarantee that those devices will work correctly. I just thinking about build in uvc compliance tester insight of the module. Some thing what users can use right in shop or at home before 14 day return guarantee. You enables compliance test and it print results in in dmesg. One of test should be header check, error/drop rate, and so on. That's an interesting idea. It should probably come with a userspace stress test software as well. -- Regards, Laurent Pinchart ___ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
Re: [Linux-uvc-devel] uvc header question
Am 31.08.2011 00:32, schrieb Laurent Pinchart: Hi Alexey, On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote: Hi Laurent, are there any reason why uvc_video_decode_start do not do precise header size checks? Are there many cameras with broken header size too? How precise do you mean ? The driver currently doesn't use much of the header, so it just makes sure that the header size is smaller than or equal to the packet size, and that it's at least 2 bytes long. I send you patch on what i work now to catch streams with fragmented packets.. what do you think about it? Will you apply some thing like this? I'm not sure about that. Webcams that would require something like that are so broken that I'm tempted to consider them as not UVC-compliant. They should be returned to vendors with a loud complaint. Your patch might help, but the sad story is that it can't completely fix the streams. There's always a chance that fragmented packets that contains no header will start with data that looks like a header. You won't be able to find a buller-proof solution. You are right, the idea is not to show definitely broken frames. If there is some thing what we can't filter, is ok. we did our best. I just thinking about build in uvc compliance tester insight of the module. Some thing what users can use right in shop or at home before 14 day return guarantee. You enables compliance test and it print results in in dmesg. One of test should be header check, error/drop rate, and so on. ___ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
Re: [Linux-uvc-devel] uvc header question
Hi Alexey, On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote: Hi Laurent, are there any reason why uvc_video_decode_start do not do precise header size checks? Are there many cameras with broken header size too? How precise do you mean ? The driver currently doesn't use much of the header, so it just makes sure that the header size is smaller than or equal to the packet size, and that it's at least 2 bytes long. I send you patch on what i work now to catch streams with fragmented packets.. what do you think about it? Will you apply some thing like this? I'm not sure about that. Webcams that would require something like that are so broken that I'm tempted to consider them as not UVC-compliant. They should be returned to vendors with a loud complaint. Your patch might help, but the sad story is that it can't completely fix the streams. There's always a chance that fragmented packets that contains no header will start with data that looks like a header. You won't be able to find a buller-proof solution. -- Regards, Laurent Pinchart ___ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
[Linux-uvc-devel] uvc header question
Hi Laurent,
are there any reason why uvc_video_decode_start do not do precise header
size checks? Are there many cameras with broken header size too?
I send you patch on what i work now to catch streams with fragmented
packets.. what do you think about it? Will you apply some thing like this?
Regards,
Alexey.
diff --git a/drivers/media/video/uvc/uvc_video.c b/drivers/media/video/uvc/uvc_video.c
index 8244167..6fd1986 100644
--- a/drivers/media/video/uvc/uvc_video.c
+++ b/drivers/media/video/uvc/uvc_video.c
@@ -371,6 +371,46 @@ int uvc_commit_video(struct uvc_streaming *stream,
#define UVC_STREAM_EOF (1 1)
#define UVC_STREAM_FID (1 0)
+static int uvc_video_jpeg_soi(const __u8 *data, int len)
+{
+ /* to check jpeg header we need minimum 2 bytes after header. */
+ if (data[0] + 2 len)
+ return 0;
+
+ if (data[data[0]] == 0xff data[data[0] + 1] == 0xd8) {
+ uvc_trace(UVC_TRACE_FRAME, jpeg_SOI signature found\n);
+ return 1;
+ } else
+ return 0;
+}
+
+static int uvc_video_jpeg_eoi(struct uvc_streaming *stream,
+ struct uvc_buffer *buf)
+{
+struct uvc_video_queue *queue = stream-queue;
+__u8 *mem, *mem_min;
+ int diff = 0;
+
+mem = queue-mem + buf-buf.m.offset + buf-buf.bytesused - 1;
+ mem_min = queue-mem + buf-buf.m.offset;
+
+ /* some times webcam can put some zeros after eoi signatur. */
+ while (*mem == 0) {
+ mem--;
+ diff++;
+ }
+
+ if (*(mem-1) == 0xff *mem == 0xd9) {
+ uvc_trace(UVC_TRACE_FRAME, jpeg_EOI signature found\n);
+ if (diff) {
+ uvc_trace(UVC_TRACE_FRAME, remove trash after EOI\n);
+ buf-buf.bytesused = buf-buf.bytesused - diff;
+ }
+ return 1;
+ } else
+ return 0;
+}
+
/* Video payload decoding is handled by uvc_video_decode_start(),
* uvc_video_decode_data() and uvc_video_decode_end().
*
@@ -409,16 +449,65 @@ int uvc_commit_video(struct uvc_streaming *stream,
static int uvc_video_decode_start(struct uvc_streaming *stream,
struct uvc_buffer *buf, const __u8 *data, int len)
{
- __u8 fid;
+ __u8 fid, calk, recover;
+ int size;
+ calk = 1;
+ recover = 0;
/* Sanity checks:
- * - packet must be at least 2 bytes long
- * - bHeaderLength value must be at least 2 bytes (see above)
- * - bHeaderLength value can't be larger than the packet size.
- */
- if (len 2 || data[0] 2 || data[0] len)
+ * - if too smal for minimal header then it is trash */
+ if (len UVC_HEADER_SIZE_MIN)
return -EINVAL;
+ /* lets do strict header check, it could be good filter, but can detect
+ * more bad but working uvc cams.
+ */
+ if (calk size != data[0]) {
+ /* calculate size of header, minimum is 2 byte */
+ size = UVC_HEADER_SIZE_MIN;
+ /* PTS should be 4 byte */
+ if (data[1] UVC_STREAM_PTS)
+ size += 4;
+ /* SCR should be 6 */
+ if (data[1] UVC_STREAM_SCR)
+ size += 6;
+ /* all together should be 12 byte */
+ if (size != data[0]) {
+ uvc_trace(UVC_TRACE_FRAME, Calculated header size
+do not match with reported:%i/%i (%i).
+Frame is corrupt or payload is fragmented.,
+size, data[0], len);
+ /* so we got some chunk of garbrage,
+ * It is against uvc specification.
+ * There are some webcams aim to be uvc but sent
+ * fragmented packets.
+ * I assume every thing more then 12 bytes can be
+ * video data. */
+ if (len UVC_HEADER_SIZE_MAX) {
+buf-error = 1;
+return 0;
+ } else
+return -EINVAL;
+ }
+ /* this is less strict check, in case PTS and/or SCR not set, but
+ * have max header leth (12) */
+ } else if (!calk
+ (data[0] UVC_HEADER_SIZE_MIN ||
+ data[0] UVC_HEADER_SIZE_MAX ||
+ data[0] len)) {
+ uvc_trace(UVC_TRACE_FRAME, Header or frame is corrupt.
+ Or payload is fragmented.);
+ if (len UVC_HEADER_SIZE_MAX) {
+ buf-error = 1;
+ return 0;
+ } else
+ return -EINVAL;
+ }
+
+ /* no need to process empty payload, exept it has EOF flag. */
+ if (data[0] == len !(data[1] UVC_STREAM_EOF))
+ return -ENODATA;
+
/* Skip payloads marked with the error bit (error frames). */
if (data[1] UVC_STREAM_ERR) {
uvc_trace(UVC_TRACE_FRAME, Dropping payload (error bit
@@ -453,6 +542,19 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
if (buf-state != UVC_BUF_STATE_ACTIVE) {
struct timespec ts;
+ if (stream-cur_format-fcc == V4L2_PIX_FMT_MJPEG) {
+ /* for mjpeg stream start new buffer only if we have
+ * valid SOI signature. */
+ if (uvc_video_jpeg_soi(data, len)) {
+stream-last_fid = -1;
+ } else {
+uvc_trace(UVC_TRACE_FRAME,
+ Dropping payload no
+ jpeg_SOI signature found. );
+return -ENODATA;
+ }
+ }
+
if (fid == stream-last_fid) {
uvc_trace(UVC_TRACE_FRAME, Dropping payload (out of
sync).\n);
@@ -493,6 +595,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
if (fid != stream-last_fid buf-buf.bytesused != 0) {
uvc_trace(UVC_TRACE_FRAME, Frame complete (FID bit
toggled).\n);
+ /* For mjpeg stream, if FID was togled but there is no SOI
+
