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
