[v1,13/19] uvcvideo: Unify UVC payload header parsing.
Commit Message
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 <posciak@chromium.org>
---
drivers/media/usb/uvc/uvc_video.c | 270 +++++++++++++++++++-------------------
drivers/media/usb/uvc/uvcvideo.h | 21 +++
2 files changed, 157 insertions(+), 134 deletions(-)
Comments
Hi Pawel,
Thank you for the patch.
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 <posciak@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)
This change (== -> <=) is unrelated. If it's need it please split it to a
separate patch.
> 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) {
> stream->stats.frame.nb_pts_diffs++;
> stream->stats.frame.last_pts_diff =
> stream->stats.frame.nb_packets;
> }
> }
>
> - if (has_pts) {
> + if (header->has_pts) {
> stream->stats.frame.nb_pts++;
> - stream->stats.frame.pts = pts;
> + stream->stats.frame.pts = header->pts;
> }
>
> /* Do all frames have a PTS in their first non-empty packet, or before
> * their first empty packet ?
> */
> if (stream->stats.frame.size == 0) {
> - if (len > header_size)
> - stream->stats.frame.has_initial_pts = has_pts;
> - if (len == header_size && has_pts)
> + if (header->payload_size > 0)
> + stream->stats.frame.has_initial_pts = header->has_pts;
> + if (header->payload_size == 0 && header->has_pts)
> stream->stats.frame.has_early_pts = true;
> }
>
> /* Do the SCR.STC and SCR.SOF fields vary through the frame ? */
> - if (has_scr && stream->stats.frame.nb_scr) {
> - if (stream->stats.frame.scr_stc != scr_stc)
> + if (header->has_scr && stream->stats.frame.nb_scr) {
> + if (stream->stats.frame.scr_stc != header->stc)
> stream->stats.frame.nb_scr_diffs++;
> }
>
> - if (has_scr) {
> + if (header->has_scr) {
> /* Expand the SOF counter to 32 bits and store its value. */
> if (stream->stats.stream.nb_frames > 0 ||
> stream->stats.frame.nb_scr > 0)
> stream->stats.stream.scr_sof_count +=
> - (scr_sof - stream->stats.stream.scr_sof) % 2048;
> - stream->stats.stream.scr_sof = scr_sof;
> + (header->sof - stream->stats.stream.scr_sof)
> + % 2048;
> + stream->stats.stream.scr_sof = header->sof;
>
> stream->stats.frame.nb_scr++;
> - stream->stats.frame.scr_stc = scr_stc;
> - stream->stats.frame.scr_sof = scr_sof;
> + stream->stats.frame.scr_stc = header->stc;
> + stream->stats.frame.scr_sof = header->sof;
>
> - if (scr_sof < stream->stats.stream.min_sof)
> - stream->stats.stream.min_sof = scr_sof;
> - if (scr_sof > stream->stats.stream.max_sof)
> - stream->stats.stream.max_sof = scr_sof;
> + if (header->sof < stream->stats.stream.min_sof)
> + stream->stats.stream.min_sof = header->sof;
> + if (header->sof > stream->stats.stream.max_sof)
> + stream->stats.stream.max_sof = header->sof;
> }
>
> /* Record the first non-empty packet number. */
> - if (stream->stats.frame.size == 0 && len > header_size)
> + if (stream->stats.frame.size == 0 && header->payload_size > 0)
> stream->stats.frame.first_data = stream->stats.frame.nb_packets;
>
> /* Update the frame size. */
> - stream->stats.frame.size += len - header_size;
> + stream->stats.frame.size += header->payload_size;
>
> /* Update the packets counters. */
> stream->stats.frame.nb_packets++;
> - if (len > header_size)
> + if (header->payload_size == 0)
This fixes a bug, could you please split it to a separate patch ? Just turn
len > header_size into len == header_size in a bugfix patch of its own, before
this one.
> stream->stats.frame.nb_empty++;
>
> - if (data[1] & UVC_STREAM_ERR)
> + if (header->has_err)
> stream->stats.frame.nb_errors++;
> }
>
> @@ -1006,21 +940,9 @@ static void uvc_video_stats_stop(struct uvc_streaming
> *stream) * uvc_video_decode_end will never be called with a NULL buffer.
> */
> static int uvc_video_decode_start(struct uvc_streaming *stream,
> - struct uvc_buffer *buf, const __u8 *data, int len)
> + struct uvc_buffer *buf, struct uvc_payload_header *header)
> {
> - __u8 fid;
> -
> - /* 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) {
> - stream->stats.frame.nb_invalid++;
> - return -EINVAL;
> - }
> -
> - fid = data[1] & UVC_STREAM_FID;
> + u8 fid = header->fid;
>
> /* Increase the sequence number regardless of any buffer states, so
> * that discontinuous sequence numbers always indicate lost frames.
> @@ -1031,8 +953,8 @@ static int uvc_video_decode_start(struct uvc_streaming
> *stream, uvc_video_stats_update(stream);
> }
>
> - uvc_video_clock_decode(stream, buf, data, len);
> - uvc_video_stats_decode(stream, data, len);
> + uvc_video_clock_decode(stream, buf, header);
> + uvc_video_stats_decode(stream, header);
>
> /* Store the payload FID bit and return immediately when the buffer is
> * NULL.
> @@ -1043,7 +965,7 @@ static int uvc_video_decode_start(struct uvc_streaming
> *stream, }
>
> /* Mark the buffer as bad if the error bit is set. */
> - if (data[1] & UVC_STREAM_ERR) {
> + if (header->has_err) {
> uvc_trace(UVC_TRACE_FRAME, "Marking buffer as bad (error bit "
> "set).\n");
> buf->error = 1;
> @@ -1064,7 +986,7 @@ static int uvc_video_decode_start(struct uvc_streaming
> *stream, uvc_trace(UVC_TRACE_FRAME, "Dropping payload (out of "
> "sync).\n");
> if ((stream->dev->quirks & UVC_QUIRK_STREAM_NO_FID) &&
> - (data[1] & UVC_STREAM_EOF))
> + (header->has_eof))
> stream->last_fid ^= UVC_STREAM_FID;
> return -ENODATA;
> }
> @@ -1107,7 +1029,7 @@ static int uvc_video_decode_start(struct uvc_streaming
> *stream,
>
> stream->last_fid = fid;
>
> - return data[0];
> + return 0;
> }
>
> static void uvc_video_decode_data(struct uvc_streaming *stream,
> @@ -1128,18 +1050,20 @@ static void uvc_video_decode_data(struct
> uvc_streaming *stream,
>
> /* Complete the current frame if the buffer size was exceeded. */
> if (len > maxlen) {
> - uvc_trace(UVC_TRACE_FRAME, "Frame complete (overflow).\n");
> + uvc_trace(UVC_TRACE_FRAME, "Frame complete (overflow) "
> + "len=%d, buffer size=%d used=%d\n",
The sizes can't be negative, please use %u.
> + len, buf->length, buf->bytesused);
Do we actually need to print that extra information ? :-) Did you find it
useful during development ?
> buf->state = UVC_BUF_STATE_READY;
> }
> }
>
> static void uvc_video_decode_end(struct uvc_streaming *stream,
> - struct uvc_buffer *buf, const __u8 *data, int len)
> + struct uvc_buffer *buf, struct uvc_payload_header *header)
> {
> /* Mark the buffer as done if the EOF marker is set. */
> - if (data[1] & UVC_STREAM_EOF && buf->bytesused != 0) {
> + if (header->has_eof && buf->bytesused != 0) {
> uvc_trace(UVC_TRACE_FRAME, "Frame complete (EOF found).\n");
> - if (data[0] == len)
> + if (header->payload_size == 0)
> uvc_trace(UVC_TRACE_FRAME, "EOF in empty payload.\n");
> buf->state = UVC_BUF_STATE_READY;
> if (stream->dev->quirks & UVC_QUIRK_STREAM_NO_FID)
> @@ -1186,6 +1110,75 @@ static int uvc_video_encode_data(struct uvc_streaming
> *stream, return nbytes;
> }
>
> +static int uvc_video_parse_header(struct uvc_streaming *stream,
> + const __u8 *data, int len, struct uvc_payload_header *header)
> +{
> + int off = 2;
The offset can't be negative, you can thus use an unsigned int type. And I
don't think there's a need to abbreviate the variable name, you can call it
offset. The lines below are not that long.
> +
> + /* Sanity checks:
> + * - packet must be at least 2 bytes long
> + * - bHeaderLength value must be at least 2 bytes (see above)
> + */
> + if (len < 2 || data[0] < 2)
> + goto error;
> +
> + header->length = 2; /* 1 byte of header length + 1 byte of BFH. */
> +
> + header->has_sli = false;
> + header->has_eof = data[1] & UVC_STREAM_EOF;
> + header->has_pts = data[1] & UVC_STREAM_PTS;
> + header->has_scr = data[1] & UVC_STREAM_SCR;
> + header->has_err = data[1] & UVC_STREAM_ERR;
> +
> + if (header->has_pts)
> + header->length += 4;
> +
> + if (header->has_scr)
> + header->length += 6;
> +
> + if (stream->cur_format->fcc == V4L2_PIX_FMT_VP8) {
> + /* VP8 payload has 2 additional bytes of BFH. */
> + header->length += 2;
> + off += 2;
> +
> + /* SLI always present for VP8 simulcast (at the end of header),
> + * allowed for VP8 non-simulcast.
> + */
> + header->has_sli = data[1] & UVC_STREAM_SLI;
> + if (header->has_sli)
> + header->length += 2;
> + }
> +
> + /* - bHeaderLength value can't be larger than the packet size. */
> + if (len < data[0] || data[0] != header->length)
Can you keep the comment and the len < data[0] check above with the other
sanity checks as in the original code ? The data[0] != header->length check
obviously needs to stay here.
> + goto error;
> +
> + /* PTS 4 bytes, STC 4 bytes, SOF 2 bytes. */
> + if (header->has_pts) {
> + header->pts = get_unaligned_le32(&data[off]);
> + off += 4;
> + }
> +
> + if (header->has_scr) {
> + header->stc = get_unaligned_le32(&data[off]);
> + off += 4;
> + header->sof = get_unaligned_le16(&data[off]);
> + off += 2;
> + }
> +
> + if (header->has_sli)
> + header->sli = get_unaligned_le16(&data[off]);
> +
> + header->payload_size = len - header->length;
> + header->fid = data[1] & UVC_STREAM_FID;
> +
> + return 0;
> +
> +error:
> + stream->stats.frame.nb_invalid++;
> + return -EINVAL;
> +}
> +
> /* ------------------------------------------------------------------------
> * URB handling
> */
> @@ -1195,9 +1188,11 @@ static int uvc_video_encode_data(struct uvc_streaming
> *stream, */
> static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming
> *stream) {
> + unsigned int len;
> u8 *mem;
> int ret, i;
> struct uvc_buffer *buf = NULL;
> + struct uvc_payload_header header;
Could you move this line to the top of the function ? I try to keep variable
declarations more or less sorted by line size, even though it seems I've
failed to do so in every location in this driver :-)
>
> for (i = 0; i < urb->number_of_packets; ++i) {
> if (urb->iso_frame_desc[i].status < 0) {
> @@ -1209,12 +1204,16 @@ static void uvc_video_decode_isoc(struct urb *urb,
> struct uvc_streaming *stream) continue;
> }
>
> - /* Decode the payload header. */
> mem = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
> + len = urb->iso_frame_desc[i].actual_length;
> +
> + ret = uvc_video_parse_header(stream, mem, len, &header);
> + if (ret < 0)
> + continue;
> +
> buf = uvc_queue_get_first_buf(&stream->queue);
> do {
> - ret = uvc_video_decode_start(stream, buf, mem,
> - urb->iso_frame_desc[i].actual_length);
> + ret = uvc_video_decode_start(stream, buf, &header);
> if (ret == -EAGAIN)
> buf = uvc_queue_next_buffer(&stream->queue,
> buf);
> @@ -1224,12 +1223,11 @@ static void uvc_video_decode_isoc(struct urb *urb,
> struct uvc_streaming *stream) continue;
>
> /* Decode the payload data. */
> - uvc_video_decode_data(stream, buf, mem + ret,
> - urb->iso_frame_desc[i].actual_length - ret);
> + uvc_video_decode_data(stream, buf, mem + header.length,
> + urb->iso_frame_desc[i].actual_length - header.length);
>
> /* Process the header again. */
> - uvc_video_decode_end(stream, buf, mem,
> - urb->iso_frame_desc[i].actual_length);
> + uvc_video_decode_end(stream, buf, &header);
>
> if (buf->state == UVC_BUF_STATE_READY) {
> if (buf->length != buf->bytesused &&
> @@ -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;
> +
> 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);
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index bca8715..b355b2c 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -453,6 +453,27 @@ struct uvc_stats_stream {
> unsigned int max_sof; /* Maximum STC.SOF value */
> };
>
> +struct uvc_payload_header {
> + bool has_eof;
> +
> + bool has_pts;
> + u32 pts;
> +
> + bool has_scr;
> + u16 sof;
> + u32 stc;
> +
> + bool has_sli;
> + u16 sli;
> +
> + u8 fid;
> +
> + bool has_err;
> +
> + int length;
> + int payload_size;
> +};
> +
> struct uvc_streaming {
> struct list_head list;
> struct uvc_device *dev;
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 <posciak@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);
@@ -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) {
stream->stats.frame.nb_pts_diffs++;
stream->stats.frame.last_pts_diff =
stream->stats.frame.nb_packets;
}
}
- if (has_pts) {
+ if (header->has_pts) {
stream->stats.frame.nb_pts++;
- stream->stats.frame.pts = pts;
+ stream->stats.frame.pts = header->pts;
}
/* Do all frames have a PTS in their first non-empty packet, or before
* their first empty packet ?
*/
if (stream->stats.frame.size == 0) {
- if (len > header_size)
- stream->stats.frame.has_initial_pts = has_pts;
- if (len == header_size && has_pts)
+ if (header->payload_size > 0)
+ stream->stats.frame.has_initial_pts = header->has_pts;
+ if (header->payload_size == 0 && header->has_pts)
stream->stats.frame.has_early_pts = true;
}
/* Do the SCR.STC and SCR.SOF fields vary through the frame ? */
- if (has_scr && stream->stats.frame.nb_scr) {
- if (stream->stats.frame.scr_stc != scr_stc)
+ if (header->has_scr && stream->stats.frame.nb_scr) {
+ if (stream->stats.frame.scr_stc != header->stc)
stream->stats.frame.nb_scr_diffs++;
}
- if (has_scr) {
+ if (header->has_scr) {
/* Expand the SOF counter to 32 bits and store its value. */
if (stream->stats.stream.nb_frames > 0 ||
stream->stats.frame.nb_scr > 0)
stream->stats.stream.scr_sof_count +=
- (scr_sof - stream->stats.stream.scr_sof) % 2048;
- stream->stats.stream.scr_sof = scr_sof;
+ (header->sof - stream->stats.stream.scr_sof)
+ % 2048;
+ stream->stats.stream.scr_sof = header->sof;
stream->stats.frame.nb_scr++;
- stream->stats.frame.scr_stc = scr_stc;
- stream->stats.frame.scr_sof = scr_sof;
+ stream->stats.frame.scr_stc = header->stc;
+ stream->stats.frame.scr_sof = header->sof;
- if (scr_sof < stream->stats.stream.min_sof)
- stream->stats.stream.min_sof = scr_sof;
- if (scr_sof > stream->stats.stream.max_sof)
- stream->stats.stream.max_sof = scr_sof;
+ if (header->sof < stream->stats.stream.min_sof)
+ stream->stats.stream.min_sof = header->sof;
+ if (header->sof > stream->stats.stream.max_sof)
+ stream->stats.stream.max_sof = header->sof;
}
/* Record the first non-empty packet number. */
- if (stream->stats.frame.size == 0 && len > header_size)
+ if (stream->stats.frame.size == 0 && header->payload_size > 0)
stream->stats.frame.first_data = stream->stats.frame.nb_packets;
/* Update the frame size. */
- stream->stats.frame.size += len - header_size;
+ stream->stats.frame.size += header->payload_size;
/* Update the packets counters. */
stream->stats.frame.nb_packets++;
- if (len > header_size)
+ if (header->payload_size == 0)
stream->stats.frame.nb_empty++;
- if (data[1] & UVC_STREAM_ERR)
+ if (header->has_err)
stream->stats.frame.nb_errors++;
}
@@ -1006,21 +940,9 @@ static void uvc_video_stats_stop(struct uvc_streaming *stream)
* uvc_video_decode_end will never be called with a NULL buffer.
*/
static int uvc_video_decode_start(struct uvc_streaming *stream,
- struct uvc_buffer *buf, const __u8 *data, int len)
+ struct uvc_buffer *buf, struct uvc_payload_header *header)
{
- __u8 fid;
-
- /* 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) {
- stream->stats.frame.nb_invalid++;
- return -EINVAL;
- }
-
- fid = data[1] & UVC_STREAM_FID;
+ u8 fid = header->fid;
/* Increase the sequence number regardless of any buffer states, so
* that discontinuous sequence numbers always indicate lost frames.
@@ -1031,8 +953,8 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
uvc_video_stats_update(stream);
}
- uvc_video_clock_decode(stream, buf, data, len);
- uvc_video_stats_decode(stream, data, len);
+ uvc_video_clock_decode(stream, buf, header);
+ uvc_video_stats_decode(stream, header);
/* Store the payload FID bit and return immediately when the buffer is
* NULL.
@@ -1043,7 +965,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
}
/* Mark the buffer as bad if the error bit is set. */
- if (data[1] & UVC_STREAM_ERR) {
+ if (header->has_err) {
uvc_trace(UVC_TRACE_FRAME, "Marking buffer as bad (error bit "
"set).\n");
buf->error = 1;
@@ -1064,7 +986,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
uvc_trace(UVC_TRACE_FRAME, "Dropping payload (out of "
"sync).\n");
if ((stream->dev->quirks & UVC_QUIRK_STREAM_NO_FID) &&
- (data[1] & UVC_STREAM_EOF))
+ (header->has_eof))
stream->last_fid ^= UVC_STREAM_FID;
return -ENODATA;
}
@@ -1107,7 +1029,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
stream->last_fid = fid;
- return data[0];
+ return 0;
}
static void uvc_video_decode_data(struct uvc_streaming *stream,
@@ -1128,18 +1050,20 @@ static void uvc_video_decode_data(struct uvc_streaming *stream,
/* Complete the current frame if the buffer size was exceeded. */
if (len > maxlen) {
- uvc_trace(UVC_TRACE_FRAME, "Frame complete (overflow).\n");
+ uvc_trace(UVC_TRACE_FRAME, "Frame complete (overflow) "
+ "len=%d, buffer size=%d used=%d\n",
+ len, buf->length, buf->bytesused);
buf->state = UVC_BUF_STATE_READY;
}
}
static void uvc_video_decode_end(struct uvc_streaming *stream,
- struct uvc_buffer *buf, const __u8 *data, int len)
+ struct uvc_buffer *buf, struct uvc_payload_header *header)
{
/* Mark the buffer as done if the EOF marker is set. */
- if (data[1] & UVC_STREAM_EOF && buf->bytesused != 0) {
+ if (header->has_eof && buf->bytesused != 0) {
uvc_trace(UVC_TRACE_FRAME, "Frame complete (EOF found).\n");
- if (data[0] == len)
+ if (header->payload_size == 0)
uvc_trace(UVC_TRACE_FRAME, "EOF in empty payload.\n");
buf->state = UVC_BUF_STATE_READY;
if (stream->dev->quirks & UVC_QUIRK_STREAM_NO_FID)
@@ -1186,6 +1110,75 @@ static int uvc_video_encode_data(struct uvc_streaming *stream,
return nbytes;
}
+static int uvc_video_parse_header(struct uvc_streaming *stream,
+ const __u8 *data, int len, struct uvc_payload_header *header)
+{
+ int off = 2;
+
+ /* Sanity checks:
+ * - packet must be at least 2 bytes long
+ * - bHeaderLength value must be at least 2 bytes (see above)
+ */
+ if (len < 2 || data[0] < 2)
+ goto error;
+
+ header->length = 2; /* 1 byte of header length + 1 byte of BFH. */
+
+ header->has_sli = false;
+ header->has_eof = data[1] & UVC_STREAM_EOF;
+ header->has_pts = data[1] & UVC_STREAM_PTS;
+ header->has_scr = data[1] & UVC_STREAM_SCR;
+ header->has_err = data[1] & UVC_STREAM_ERR;
+
+ if (header->has_pts)
+ header->length += 4;
+
+ if (header->has_scr)
+ header->length += 6;
+
+ if (stream->cur_format->fcc == V4L2_PIX_FMT_VP8) {
+ /* VP8 payload has 2 additional bytes of BFH. */
+ header->length += 2;
+ off += 2;
+
+ /* SLI always present for VP8 simulcast (at the end of header),
+ * allowed for VP8 non-simulcast.
+ */
+ header->has_sli = data[1] & UVC_STREAM_SLI;
+ if (header->has_sli)
+ header->length += 2;
+ }
+
+ /* - bHeaderLength value can't be larger than the packet size. */
+ if (len < data[0] || data[0] != header->length)
+ goto error;
+
+ /* PTS 4 bytes, STC 4 bytes, SOF 2 bytes. */
+ if (header->has_pts) {
+ header->pts = get_unaligned_le32(&data[off]);
+ off += 4;
+ }
+
+ if (header->has_scr) {
+ header->stc = get_unaligned_le32(&data[off]);
+ off += 4;
+ header->sof = get_unaligned_le16(&data[off]);
+ off += 2;
+ }
+
+ if (header->has_sli)
+ header->sli = get_unaligned_le16(&data[off]);
+
+ header->payload_size = len - header->length;
+ header->fid = data[1] & UVC_STREAM_FID;
+
+ return 0;
+
+error:
+ stream->stats.frame.nb_invalid++;
+ return -EINVAL;
+}
+
/* ------------------------------------------------------------------------
* URB handling
*/
@@ -1195,9 +1188,11 @@ static int uvc_video_encode_data(struct uvc_streaming *stream,
*/
static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream)
{
+ unsigned int len;
u8 *mem;
int ret, i;
struct uvc_buffer *buf = NULL;
+ struct uvc_payload_header header;
for (i = 0; i < urb->number_of_packets; ++i) {
if (urb->iso_frame_desc[i].status < 0) {
@@ -1209,12 +1204,16 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream)
continue;
}
- /* Decode the payload header. */
mem = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
+ len = urb->iso_frame_desc[i].actual_length;
+
+ ret = uvc_video_parse_header(stream, mem, len, &header);
+ if (ret < 0)
+ continue;
+
buf = uvc_queue_get_first_buf(&stream->queue);
do {
- ret = uvc_video_decode_start(stream, buf, mem,
- urb->iso_frame_desc[i].actual_length);
+ ret = uvc_video_decode_start(stream, buf, &header);
if (ret == -EAGAIN)
buf = uvc_queue_next_buffer(&stream->queue,
buf);
@@ -1224,12 +1223,11 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream)
continue;
/* Decode the payload data. */
- uvc_video_decode_data(stream, buf, mem + ret,
- urb->iso_frame_desc[i].actual_length - ret);
+ uvc_video_decode_data(stream, buf, mem + header.length,
+ urb->iso_frame_desc[i].actual_length - header.length);
/* Process the header again. */
- uvc_video_decode_end(stream, buf, mem,
- urb->iso_frame_desc[i].actual_length);
+ uvc_video_decode_end(stream, buf, &header);
if (buf->state == UVC_BUF_STATE_READY) {
if (buf->length != buf->bytesused &&
@@ -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;
+
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);
@@ -453,6 +453,27 @@ struct uvc_stats_stream {
unsigned int max_sof; /* Maximum STC.SOF value */
};
+struct uvc_payload_header {
+ bool has_eof;
+
+ bool has_pts;
+ u32 pts;
+
+ bool has_scr;
+ u16 sof;
+ u32 stc;
+
+ bool has_sli;
+ u16 sli;
+
+ u8 fid;
+
+ bool has_err;
+
+ int length;
+ int payload_size;
+};
+
struct uvc_streaming {
struct list_head list;
struct uvc_device *dev;