[FFmpeg-devel] Patches related to bug 4988

Submitted by Oliver Collyer on Sept. 10, 2016, 8:22 a.m.

Details

Message ID 9444DC78-7A1D-4016-B6FF-9BF96F4579CE@mac.com
State New
Headers show

Commit Message

Oliver Collyer Sept. 10, 2016, 8:22 a.m.
So I’ve spent some more time investigating this issue.

It actually affects the two USB capture devices I have, the Magewell XI100DUSB-HDMI and the Inogeni DVI2USB.

Each of them suffers similar (but not identical) problems with corrupted buffers at the start of the capture.

One workaround is to issue “modprobe -r uvcvideo && modprobe uvcvideo” to re-load the uvcvideo module each time before running FFmpeg. This works for both devices and the captures then proceed as normal.

Alternatively, below is a patch for FFmpeg that also deals with the problem.

Currently when ffmpeg encounters the error flag it just sets the buffers “bytesused” to zero and carries on as normal. The trouble with this is that these corrupted buffers have a zero timestamp so it leads to FFmpeg spewing out warnings for the duration of the capture once the non-corrupted buffers arrive with normal timestamps, due to the difference between them. So the first thing the patch does is discard the corrupted buffers rather than attempting to use them. I think this is a more sensible approach anyway - it seems counterproductive to try and handle a buffer as normal that we have already been told is corrupt.

Unfortunately this doesn’t completely solve the problem because even if we discard the corrupted buffers it seems that for a short time afterwards the valid buffers don’t arrive at the usual regular intervals, so you can still get many warnings inside FFmpeg due to the initial irregular timestamps. Both devices exhibit the same behaviour and it reproduces on different machines. So the other thing the patch does is ignore a fixed number of valid buffers following the last corrupted one. I found 3-4 buffers is usually enough to ignore, but to be on the safe side I have set this to 8. This solves the issues and prevents any more warnings, but I don’t really think hardcoding a number of buffers to ignore is really a clean solution is it? Does anyone have any better ideas? Maybe this behaviour that ignores buffers should be an argument? Maybe it should only do this at the start of the capture and not after corrupted buffers encountered once valid buffers have been received?

As mentioned in my earlier message I had also written a patch adding an option “-timestamps discard” to discard the timestamps altogether as another solution but this seemed like overkill as once the capture has “settled down” all the timestamps are just fine.

I’ve also been trying to get the underlying issue fixed by the v4l2/linuxtv developers, and also attempted to involve the manufacturers of these devices, but nobody has really been able to look at it properly yet.

Anyway, patch below.

Comments

Carl Eugen Hoyos Sept. 20, 2016, 3:51 p.m.
2016-09-10 10:22 GMT+02:00 Oliver Collyer <ovcollyer@mac.com>:

> One workaround is to issue
> “modprobe -r uvcvideo && modprobe uvcvideo” to re-load the
> uvcvideo module each time before running FFmpeg. This works
> for both devices and the captures then proceed as normal.

Doesn't this indicate that there really is no issue in FFmpeg to fix?

Do you think that your patch has any side-effects?

Carl Eugen

Patch hide | download patch | download mbox

diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
old mode 100644
new mode 100755
index ddf331d..7b4a826
--- a/libavdevice/v4l2.c
+++ b/libavdevice/v4l2.c
@@ -79,6 +79,7 @@  struct video_data {
 
     int buffers;
     volatile int buffers_queued;
+    int buffers_ignore;
     void **buf_start;
     unsigned int *buf_len;
     char *standard;
@@ -519,7 +520,9 @@  static int mmap_read_frame(AVFormatContext *ctx, AVPacket *pkt)
         av_log(ctx, AV_LOG_WARNING,
                "Dequeued v4l2 buffer contains corrupted data (%d bytes).\n",
                buf.bytesused);
-        buf.bytesused = 0;
+        s->buffers_ignore = 8;
+        enqueue_buffer(s, &buf);
+        return FFERROR_REDO;
     } else
 #endif
     {
@@ -529,14 +532,28 @@  static int mmap_read_frame(AVFormatContext *ctx, AVPacket *pkt)
             s->frame_size = buf.bytesused;
 
         if (s->frame_size > 0 && buf.bytesused != s->frame_size) {
-            av_log(ctx, AV_LOG_ERROR,
+            av_log(ctx, AV_LOG_WARNING,
                    "Dequeued v4l2 buffer contains %d bytes, but %d were expected. Flags: 0x%08X.\n",
                    buf.bytesused, s->frame_size, buf.flags);
+            s->buffers_ignore = 8;
             enqueue_buffer(s, &buf);
-            return AVERROR_INVALIDDATA;
+            return FFERROR_REDO;
         }
     }
 
+    
+    /* if we just encounted some corrupted buffers then we ignore the next few
+     * legitimate buffers because they can arrive at irregular intervals, causing
+     * the timestamps of the input and output streams to be out-of-sync and FFmpeg
+     * to continually emit warnings. */
+    if (s->buffers_ignore) {
+        av_log(ctx, AV_LOG_WARNING,
+               "Ignoring dequeued v4l2 buffer due to earlier corruption.\n");
+        s->buffers_ignore --;
+        enqueue_buffer(s, &buf);
+        return FFERROR_REDO;
+    }
+
     /* Image is at s->buff_start[buf.index] */
     if (avpriv_atomic_int_get(&s->buffers_queued) == FFMAX(s->buffers / 8, 1)) {
         /* when we start getting low on queued buffers, fall back on copying data */
@@ -608,6 +625,7 @@  static int mmap_start(AVFormatContext *ctx)
         }
     }
     s->buffers_queued = s->buffers;
+    s->buffers_ignore = 0;
 
     type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
     if (v4l2_ioctl(s->fd, VIDIOC_STREAMON, &type) < 0) {