Hello,
as Reimar pointed out the proposed patch is not the correct solution.
Please see/test attached patch.
Regards,
Giorgio Vazzana
2014-08-31 14:35 GMT+02:00 Dmitry Volyntsev <xeioexcept...@gmail.com>:
>>why is this condition true ?
>
> I tried several configuration and problem occurred only under certain
> circumstances:
> 1. webcam type: Logitech C310 (tried also: C350)
> 2. capturing settings: 640*480, 422p
> 3. simultaneous capturing from two webcams
> 4. relatively old laptop (Lenovo Z470, Z570, L420)
>
> I think it somehow relate to usb hub bandwidth and laptop performance
> issue (and maybe length of usb cable?)
>
> I use my own app (24/7 surveillance home recording) which depends on
> libavdevice and prefer to ignore AVERROR_INVALIDDATA in this case
> (just skip the broken frame and continue) because reconnect to devices
> required more than a half of second. With my patch everything goes
> fine (more than 2 week of continuous recording)
>
> logs:
> [video4linux2,v4l2 @ 0x2efec00]The v4l2 frame is 614396 bytes, but
> 614400 bytes are expected
> [video4linux2,v4l2 @ 0x2efec00]The v4l2 frame is 614396 bytes, but
> 614400 bytes are expected
>
>
>
> On Sat, Aug 30, 2014 at 9:17 PM, Michael Niedermayer <michae...@gmx.at> wrote:
>> On Sat, Aug 30, 2014 at 08:19:37PM +0400, Dmitry Volyntsev wrote:
>>> To understand the problem one needs to see the original code around
>>> and to think what would happen if from time to time while capturing
>>> condition (s->frame_size > 0 && buf.bytesused != s->frame_size)
>>> happens to be true (this is not critical error so capturing would
>>> continue). It is obvious that eventually buffers_queued would become <
>>> 1.
>>
>> why is this condition true, what are these mismatching buffers ?
>>
>>
>>>
>>> static int mmap_read_frame(AVFormatContext *ctx, AVPacket *pkt)
>>> {
>>> ...
>>> if (buf.index >= s->buffers) {
>>> av_log(ctx, AV_LOG_ERROR, "Invalid buffer index received.\n");
>>>
>>> return AVERROR(EINVAL);
>>> }
>>>
>>> avpriv_atomic_int_add_and_fetch(&s->buffers_queued, -1);
>>> // always keep at least one buffer queued
>>> av_assert0(avpriv_atomic_int_get(&s->buffers_queued) >= 1);
>>>
>>> if (s->frame_size > 0 && buf.bytesused != s->frame_size) {
>>> av_log(ctx, AV_LOG_ERROR,
>>> "The v4l2 frame is %d bytes, but %d bytes are expected\n",
>>> buf.bytesused, s->frame_size);
>>> return AVERROR_INVALIDDATA;
>>>
>>> }
>>>
>>> So my solution: we should make all checks here before decrementing
>>> buffers_queued
>>>
>>> On Wed, Aug 27, 2014 at 1:20 AM, Michael Niedermayer <michae...@gmx.at>
>>> wrote:
>>> > On Wed, Aug 13, 2014 at 07:04:01PM +0400, Dmitry Volyntsev wrote:
>>> >> From: Dmitry Volyntsev <xeioexcept...@gmail.com>
>>> >>
>>> >> s->buffers_queued constantly decremented and not incremented
>>> >> in case of (s->frame_size > 0 && buf.bytesused != s->frame_size)
>>> >> condition (caught on long run capture of Logitech C310)
>>> >
>>> > can you explain why this happens ? where do this misatching
>>> > bufs come from ?
>>> > why is droping them correct ?
>>> >
>>> >
>>> > [...]
>>> > --
>>> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>> >
>>> > Dictatorship naturally arises out of democracy, and the most aggravated
>>> > form of tyranny and slavery out of the most extreme liberty. -- Plato
>>>
>>>
>>>
>>> --
>>> Be happy,
>>> Best regards,
>>> Dmitry Volyntsev
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> In a rich man's house there is no place to spit but his face.
>> -- Diogenes of Sinope
>
>
>
> --
> Be happy,
> Best regards,
> Dmitry Volyntsev
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From 241b1310bf7925a62bccba2151c2ca5d2bc61f15 Mon Sep 17 00:00:00 2001
From: Giorgio Vazzana <mywin...@gmail.com>
Date: Mon, 1 Sep 2014 22:20:05 +0200
Subject: [PATCH] lavd/v4l2: introduce enqueue_buffer()
Additionally, make sure a buffer gets enqueued again (even in error paths) after
it has been succesfully dequeued.
---
libavdevice/v4l2.c | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
index 9f9f944..b55052a 100644
--- a/libavdevice/v4l2.c
+++ b/libavdevice/v4l2.c
@@ -403,10 +403,23 @@ static void dummy_release_buffer(AVPacket *pkt)
}
#endif
+static int enqueue_buffer(struct video_data *s, struct v4l2_buffer *buf)
+{
+ int res = 0;
+
+ if (v4l2_ioctl(s->fd, VIDIOC_QBUF, buf) < 0) {
+ res = AVERROR(errno);
+ av_log(NULL, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n", av_err2str(res));
+ } else {
+ avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
+ }
+
+ return res;
+}
+
static void mmap_release_buffer(void *opaque, uint8_t *data)
{
struct v4l2_buffer buf = { 0 };
- int res;
struct buff_data *buf_descriptor = opaque;
struct video_data *s = buf_descriptor->s;
@@ -415,13 +428,7 @@ static void mmap_release_buffer(void *opaque, uint8_t *data)
buf.index = buf_descriptor->index;
av_free(buf_descriptor);
- if (v4l2_ioctl(s->fd, VIDIOC_QBUF, &buf) < 0) {
- res = AVERROR(errno);
- av_log(NULL, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n",
- av_err2str(res));
- }
-
- avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
+ enqueue_buffer(s, &buf);
}
#if HAVE_CLOCK_GETTIME && defined(CLOCK_MONOTONIC)
@@ -524,6 +531,7 @@ static int mmap_read_frame(AVFormatContext *ctx, AVPacket *pkt)
av_log(ctx, AV_LOG_ERROR,
"The v4l2 frame is %d bytes, but %d bytes are expected\n",
buf.bytesused, s->frame_size);
+ enqueue_buffer(s, &buf);
return AVERROR_INVALIDDATA;
}
@@ -533,19 +541,16 @@ static int mmap_read_frame(AVFormatContext *ctx, AVPacket *pkt)
res = av_new_packet(pkt, buf.bytesused);
if (res < 0) {
av_log(ctx, AV_LOG_ERROR, "Error allocating a packet.\n");
- if (v4l2_ioctl(s->fd, VIDIOC_QBUF, &buf) == 0)
- avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
+ enqueue_buffer(s, &buf);
return res;
}
memcpy(pkt->data, s->buf_start[buf.index], buf.bytesused);
- if (v4l2_ioctl(s->fd, VIDIOC_QBUF, &buf) < 0) {
- res = AVERROR(errno);
- av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n", av_err2str(res));
+ res = enqueue_buffer(s, &buf);
+ if (res) {
av_free_packet(pkt);
return res;
}
- avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
} else {
struct buff_data *buf_descriptor;
@@ -563,8 +568,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
* allocate a buffer for memcpying into it
*/
av_log(ctx, AV_LOG_ERROR, "Failed to allocate a buffer descriptor\n");
- if (v4l2_ioctl(s->fd, VIDIOC_QBUF, &buf) == 0)
- avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
+ enqueue_buffer(s, &buf);
return AVERROR(ENOMEM);
}
@@ -575,8 +579,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
buf_descriptor, 0);
if (!pkt->buf) {
av_log(ctx, AV_LOG_ERROR, "Failed to create a buffer\n");
- if (v4l2_ioctl(s->fd, VIDIOC_QBUF, &buf) == 0)
- avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
+ enqueue_buffer(s, &buf);
av_freep(&buf_descriptor);
return AVERROR(ENOMEM);
}
--
1.7.9.5
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
