Re: [PATCH v1 13/19] uvcvideo: Unify UVC payload header parsing.
Hi Pawel,
One more comment.
On Friday 30 August 2013 11:17:12 Pawel Osciak wrote:
Create a separate function for parsing UVC payload headers and extract code
from other functions into it. Store the parsed values in a header struct.
Signed-off-by: Pawel Osciak posc...@chromium.org
---
drivers/media/usb/uvc/uvc_video.c | 270 +--
drivers/media/usb/uvc/uvcvideo.h | 21 +++
2 files changed, 157 insertions(+), 134 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c index 2f9a5fa..59f57a2 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
[snip]
@@ -1246,6 +1244,7 @@ static void uvc_video_decode_bulk(struct urb *urb,
struct uvc_streaming *stream) {
u8 *mem;
int len, ret;
+ struct uvc_payload_header header;
struct uvc_buffer *buf;
/*
@@ -1259,6 +1258,10 @@ static void uvc_video_decode_bulk(struct urb *urb,
struct uvc_streaming *stream) len = urb-actual_length;
stream-bulk.payload_size += len;
+ ret = uvc_video_parse_header(stream, mem, len, header);
+ if (ret 0)
+ return;
+
This won't work. UVC transmits a single header per payload and splits the
payload to multiple URBs in the case of bulk transfers. Only the first URB
will thus have a header. You should parse the payload inside the if { ... } in
the next hunk, and save the header (or at least the needed fields) in stream-
bulk for the uvc_video_decode_end() call.
buf = uvc_queue_get_first_buf(stream-queue);
/* If the URB is the first of its payload, decode and save the
@@ -1266,7 +1269,7 @@ static void uvc_video_decode_bulk(struct urb *urb,
struct uvc_streaming *stream) */
if (stream-bulk.header_size == 0 !stream-bulk.skip_payload) {
do {
- ret = uvc_video_decode_start(stream, buf, mem, len);
+ ret = uvc_video_decode_start(stream, buf, header);
if (ret == -EAGAIN)
buf = uvc_queue_next_buffer(stream-queue,
buf);
@@ -1276,11 +1279,11 @@ static void uvc_video_decode_bulk(struct urb *urb,
struct uvc_streaming *stream) if (ret 0 || buf == NULL) {
stream-bulk.skip_payload = 1;
} else {
- memcpy(stream-bulk.header, mem, ret);
- stream-bulk.header_size = ret;
+ memcpy(stream-bulk.header, mem, header.length);
+ stream-bulk.header_size = header.length;
- mem += ret;
- len -= ret;
+ mem += header.length;
+ len -= header.length;
}
}
@@ -1299,8 +1302,7 @@ static void uvc_video_decode_bulk(struct urb *urb,
struct uvc_streaming *stream) if (urb-actual_length
urb-transfer_buffer_length ||
stream-bulk.payload_size = stream-bulk.max_payload_size) {
if (!stream-bulk.skip_payload buf != NULL) {
- uvc_video_decode_end(stream, buf, stream-bulk.header,
- stream-bulk.payload_size);
+ uvc_video_decode_end(stream, buf, header);
if (buf-state == UVC_BUF_STATE_READY)
buf = uvc_queue_next_buffer(stream-queue,
buf);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 13/19] uvcvideo: Unify UVC payload header parsing.
Create a separate function for parsing UVC payload headers and extract code
from other functions into it. Store the parsed values in a header struct.
Signed-off-by: Pawel Osciak posc...@chromium.org
---
drivers/media/usb/uvc/uvc_video.c | 270 +++---
drivers/media/usb/uvc/uvcvideo.h | 21 +++
2 files changed, 157 insertions(+), 134 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 2f9a5fa..59f57a2 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -422,40 +422,14 @@ static int uvc_commit_video(struct uvc_streaming *stream,
static void
uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
- const __u8 *data, int len)
+ struct uvc_payload_header *header)
{
struct uvc_clock_sample *sample;
- unsigned int header_size;
- bool has_pts = false;
- bool has_scr = false;
unsigned long flags;
struct timespec ts;
u16 host_sof;
u16 dev_sof;
- switch (data[1] (UVC_STREAM_PTS | UVC_STREAM_SCR)) {
- case UVC_STREAM_PTS | UVC_STREAM_SCR:
- header_size = 12;
- has_pts = true;
- has_scr = true;
- break;
- case UVC_STREAM_PTS:
- header_size = 6;
- has_pts = true;
- break;
- case UVC_STREAM_SCR:
- header_size = 8;
- has_scr = true;
- break;
- default:
- header_size = 2;
- break;
- }
-
- /* Check for invalid headers. */
- if (len header_size)
- return;
-
/* Extract the timestamps:
*
* - store the frame PTS in the buffer structure
@@ -463,17 +437,17 @@ uvc_video_clock_decode(struct uvc_streaming *stream,
struct uvc_buffer *buf,
* kernel timestamps and store them with the SCR STC and SOF fields
* in the ring buffer
*/
- if (has_pts buf != NULL)
- buf-pts = get_unaligned_le32(data[2]);
+ if (header-has_pts buf != NULL)
+ buf-pts = header-pts;
- if (!has_scr)
+ if (!header-has_scr)
return;
/* To limit the amount of data, drop SCRs with an SOF identical to the
* previous one.
*/
- dev_sof = get_unaligned_le16(data[header_size - 2]);
- if (dev_sof == stream-clock.last_sof)
+ dev_sof = header-sof;
+ if (dev_sof = stream-clock.last_sof)
return;
stream-clock.last_sof = dev_sof;
@@ -513,7 +487,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct
uvc_buffer *buf,
spin_lock_irqsave(stream-clock.lock, flags);
sample = stream-clock.samples[stream-clock.head];
- sample-dev_stc = get_unaligned_le32(data[header_size - 6]);
+ sample-dev_stc = header-stc;
sample-dev_sof = dev_sof;
sample-host_sof = host_sof;
sample-host_ts = ts;
@@ -756,114 +730,74 @@ done:
*/
static void uvc_video_stats_decode(struct uvc_streaming *stream,
- const __u8 *data, int len)
+ struct uvc_payload_header *header)
{
- unsigned int header_size;
- bool has_pts = false;
- bool has_scr = false;
- u16 uninitialized_var(scr_sof);
- u32 uninitialized_var(scr_stc);
- u32 uninitialized_var(pts);
-
if (stream-stats.stream.nb_frames == 0
stream-stats.frame.nb_packets == 0)
ktime_get_ts(stream-stats.stream.start_ts);
- switch (data[1] (UVC_STREAM_PTS | UVC_STREAM_SCR)) {
- case UVC_STREAM_PTS | UVC_STREAM_SCR:
- header_size = 12;
- has_pts = true;
- has_scr = true;
- break;
- case UVC_STREAM_PTS:
- header_size = 6;
- has_pts = true;
- break;
- case UVC_STREAM_SCR:
- header_size = 8;
- has_scr = true;
- break;
- default:
- header_size = 2;
- break;
- }
-
- /* Check for invalid headers. */
- if (len header_size || data[0] header_size) {
- stream-stats.frame.nb_invalid++;
- return;
- }
-
- /* Extract the timestamps. */
- if (has_pts)
- pts = get_unaligned_le32(data[2]);
-
- if (has_scr) {
- scr_stc = get_unaligned_le32(data[header_size - 6]);
- scr_sof = get_unaligned_le16(data[header_size - 2]);
- }
-
/* Is PTS constant through the whole frame ? */
- if (has_pts stream-stats.frame.nb_pts) {
- if (stream-stats.frame.pts != pts) {
+ if (header-has_pts stream-stats.frame.nb_pts) {
+ if (stream-stats.frame.pts != header-pts) {
