Re: [FFmpeg-devel] Patches related to bug 4988

2016-09-20 Thread Carl Eugen Hoyos
2016-09-10 10:22 GMT+02:00 Oliver Collyer :

> 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Patches related to bug 4988

2016-09-10 Thread Oliver Collyer
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.

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, );
+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, );
-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, );
+return FFERROR_REDO;
+}
+
 /* Image is at s->buff_start[buf.index] */
 if (avpriv_atomic_int_get(>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