umlaeute / v4l2loopback

v4l2-loopback device
GNU General Public License v2.0
3.61k stars 515 forks source link

gst-launch-1.0 stops at first frame #535

Closed sanbrother closed 1 year ago

sanbrother commented 1 year ago

Environment

Problem:

gst-launch-1.0 stops at first frame, seems freezed.

Steps to reproduce:

sudo modprobe v4l2loopback max_buffers=8
gst-launch-1.0 -v videotestsrc ! v4l2sink device=/dev/video0
# launch VLC Media Player
vlc -d v4l2:///dev/video0

Relevant Code:

I'm sure, the commit https://github.com/umlaeute/v4l2loopback/commit/ff4e9ee3510109333ace2645ff5b7d1f7edb36d9 introduced a bug.

Here is my patch, strange, but works. As far as I understand, write_position and read_position should not be applied modulo operation.

index 1a02f57..9b04506 100644
--- a/v4l2loopback.c
+++ b/v4l2loopback.c
@@ -96,10 +96,10 @@ MODULE_LICENSE("GPL");
 static inline int mod_inc(int *number, int mod)
 {
        int result;
-       result = (*number + 1) % mod;
+       result = (*number) % mod;
        if (unlikely(result < 0))
                result += mod;
-       *number = result;
+       *number = (*number + 1);
        return result;
 }
betaboon commented 1 year ago

i just ran into the same issue. running a git bisect between v0.12.7 and current HEAD also shows that the mentioned commit is the culprit. i then tested with a revert-commit and the problem goes away.

this is the revert-patch:

diff --git a/v4l2loopback.c b/v4l2loopback.c
index 2ab1f76..2514f09 100644
--- a/v4l2loopback.c
+++ b/v4l2loopback.c
@@ -92,17 +92,6 @@ MODULE_LICENSE("GPL");
        }                                                      \
    } while (0)

-/* TODO: Make sure that function is never interrupted. */
-static inline int mod_inc(int *number, int mod)
-{
-   int result;
-   result = (*number + 1) % mod;
-   if (unlikely(result < 0))
-       result += mod;
-   *number = result;
-   return result;
-}
-
 static inline void v4l2l_get_timestamp(struct v4l2_buffer *b)
 {
    /* ktime_get_ts is considered deprecated, so use ktime_get_ts64 if possible */
@@ -1424,8 +1413,9 @@ static int vidioc_reqbufs(struct file *file, void *fh,
            i = dev->write_position;
            list_for_each_entry(pos, &dev->outbufs_list,
                        list_head) {
-               dev->bufpos2index[mod_inc(&i, b->count)] =
+               dev->bufpos2index[i % b->count] =
                    pos->buffer.index;
+               ++i;
            }
        }

@@ -1489,9 +1479,10 @@ static void buffer_written(struct v4l2_loopback_device *dev,
    del_timer_sync(&dev->timeout_timer);
    spin_lock_bh(&dev->lock);

-   dev->bufpos2index[mod_inc(&dev->write_position, dev->used_buffers)] =
+   dev->bufpos2index[dev->write_position % dev->used_buffers] =
        buf->buffer.index;
    list_move_tail(&buf->list_head, &dev->outbufs_list);
+   ++dev->write_position;
    dev->reread_count = 0;

    check_timers(dev);
@@ -1586,7 +1577,8 @@ static int get_capture_buffer(struct file *file)
        if (dev->write_position >
            opener->read_position + dev->used_buffers)
            opener->read_position = dev->write_position - 1;
-       pos = mod_inc(&opener->read_position, dev->used_buffers);
+       pos = opener->read_position % dev->used_buffers;
+       ++opener->read_position;
    }
    timeout_happened = dev->timeout_happened;
    dev->timeout_happened = 0;
sanbrother commented 1 year ago

i just ran into the same issue. running a git bisect between v0.12.7 and current HEAD also shows that the mentioned commit is the culprit. i then tested with a revert-commit and the problem goes away.

It seems that, the base code depends heavily on read_position and write_position. If modulo operation applied on the two variables, it is hard to judge if (write_position > read_position) is true. So, as you see, my patch just increase the input number, but returns a modulo value. But it is strange to read the code.