Skip to content

Commit 47ca010

Browse files
ribaldagregkh
authored andcommitted
media: uvcvideo: Fix sequence number when no EOF
commit f078966 upstream. If the driver could not detect the EOF, the sequence number is increased twice: 1) When we enter uvc_video_decode_start() with the old buffer and FID has flipped => We return -EAGAIN and last_fid is not flipped 2) When we enter uvc_video_decode_start() with the new buffer. Fix this issue by moving the new frame detection logic earlier in uvc_video_decode_start(). This also has some nice side affects: - The error status from the new packet will no longer get propagated to the previous frame-buffer. - uvc_video_clock_decode() will no longer update the previous frame buf->stf with info from the new packet. - uvc_video_clock_decode() and uvc_video_stats_decode() will no longer get called twice for the same packet. Cc: stable@kernel.org Fixes: 650b95f ("[media] uvcvideo: Generate discontinuous sequence numbers when frames are lost") Reported-by: Hans de Goede <hansg@kernel.org> Closes: https://lore.kernel.org/linux-media/CANiDSCuj4cPuB5_v2xyvAagA5FjoN8V5scXiFFOeD3aKDMqkCg@mail.gmail.com/T/#me39fb134e8c2c085567a31548c3403eb639625e4 Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent e1052f8 commit 47ca010

1 file changed

Lines changed: 47 additions & 45 deletions

File tree

drivers/media/usb/uvc/uvc_video.c

Lines changed: 47 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,53 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
11431143
header_len = data[0];
11441144
fid = data[1] & UVC_STREAM_FID;
11451145

1146+
/*
1147+
* Mark the buffer as done if we're at the beginning of a new frame.
1148+
* End of frame detection is better implemented by checking the EOF
1149+
* bit (FID bit toggling is delayed by one frame compared to the EOF
1150+
* bit), but some devices don't set the bit at end of frame (and the
1151+
* last payload can be lost anyway). We thus must check if the FID has
1152+
* been toggled.
1153+
*
1154+
* stream->last_fid is initialized to -1, and buf->bytesused to 0,
1155+
* so the first isochronous frame will never trigger an end of frame
1156+
* detection.
1157+
*
1158+
* Empty buffers (bytesused == 0) don't trigger end of frame detection
1159+
* as it doesn't make sense to return an empty buffer. This also
1160+
* avoids detecting end of frame conditions at FID toggling if the
1161+
* previous payload had the EOF bit set.
1162+
*/
1163+
if (fid != stream->last_fid && buf && buf->bytesused != 0) {
1164+
uvc_dbg(stream->dev, FRAME,
1165+
"Frame complete (FID bit toggled)\n");
1166+
buf->state = UVC_BUF_STATE_READY;
1167+
1168+
return -EAGAIN;
1169+
}
1170+
1171+
/*
1172+
* Some cameras, when running two parallel streams (one MJPEG alongside
1173+
* another non-MJPEG stream), are known to lose the EOF packet for a frame.
1174+
* We can detect the end of a frame by checking for a new SOI marker, as
1175+
* the SOI always lies on the packet boundary between two frames for
1176+
* these devices.
1177+
*/
1178+
if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
1179+
(stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
1180+
stream->cur_format->fcc == V4L2_PIX_FMT_JPEG) &&
1181+
buf && buf->bytesused != 0) {
1182+
const u8 *packet = data + header_len;
1183+
1184+
if (len >= header_len + 2 &&
1185+
packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI) {
1186+
buf->state = UVC_BUF_STATE_READY;
1187+
buf->error = 1;
1188+
stream->last_fid ^= UVC_STREAM_FID;
1189+
return -EAGAIN;
1190+
}
1191+
}
1192+
11461193
/*
11471194
* Increase the sequence number regardless of any buffer states, so
11481195
* that discontinuous sequence numbers always indicate lost frames.
@@ -1210,51 +1257,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
12101257
meta_buf->state = UVC_BUF_STATE_ACTIVE;
12111258
}
12121259

1213-
/*
1214-
* Mark the buffer as done if we're at the beginning of a new frame.
1215-
* End of frame detection is better implemented by checking the EOF
1216-
* bit (FID bit toggling is delayed by one frame compared to the EOF
1217-
* bit), but some devices don't set the bit at end of frame (and the
1218-
* last payload can be lost anyway). We thus must check if the FID has
1219-
* been toggled.
1220-
*
1221-
* stream->last_fid is initialized to -1, so the first isochronous
1222-
* frame will never trigger an end of frame detection.
1223-
*
1224-
* Empty buffers (bytesused == 0) don't trigger end of frame detection
1225-
* as it doesn't make sense to return an empty buffer. This also
1226-
* avoids detecting end of frame conditions at FID toggling if the
1227-
* previous payload had the EOF bit set.
1228-
*/
1229-
if (fid != stream->last_fid && buf->bytesused != 0) {
1230-
uvc_dbg(stream->dev, FRAME,
1231-
"Frame complete (FID bit toggled)\n");
1232-
buf->state = UVC_BUF_STATE_READY;
1233-
return -EAGAIN;
1234-
}
1235-
1236-
/*
1237-
* Some cameras, when running two parallel streams (one MJPEG alongside
1238-
* another non-MJPEG stream), are known to lose the EOF packet for a frame.
1239-
* We can detect the end of a frame by checking for a new SOI marker, as
1240-
* the SOI always lies on the packet boundary between two frames for
1241-
* these devices.
1242-
*/
1243-
if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
1244-
(stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
1245-
stream->cur_format->fcc == V4L2_PIX_FMT_JPEG)) {
1246-
const u8 *packet = data + header_len;
1247-
1248-
if (len >= header_len + 2 &&
1249-
packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI &&
1250-
buf->bytesused != 0) {
1251-
buf->state = UVC_BUF_STATE_READY;
1252-
buf->error = 1;
1253-
stream->last_fid ^= UVC_STREAM_FID;
1254-
return -EAGAIN;
1255-
}
1256-
}
1257-
12581260
stream->last_fid = fid;
12591261

12601262
return header_len;

0 commit comments

Comments
 (0)