qv4l2: fix error handling and initialization bugs

From: Hans Verkuil <hverkuil@xs4all.nl>

The streaming I/O detection was flawed when used with videobuf. The
v4l2_buffer struct could contain uninitialized fields that would result
in videobuf errors. The application did not check for qbuf/dqbuf error
codes.

Priority: normal

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>

diff --git a/utils/qv4l2-qt4/general-tab.cpp b/utils/qv4l2-qt4/general-tab.cpp
index 490b565..50a3edc 100644 (file)
--- a/utils/qv4l2-qt4/general-tab.cpp
+++ b/utils/qv4l2-qt4/general-tab.cpp
@@ -198,10 +198,10 @@ GeneralTab::GeneralTab(const QString &device, v4l2 &fd, int n, QWidget *parent)
        if (m_querycap.capabilities & V4L2_CAP_STREAMING) {
                v4l2_requestbuffers reqbuf;
 
-               if (reqbufs_user_cap(reqbuf))
+               if (reqbufs_user_cap(reqbuf, 1))
                        m_capMethods->addItem("User pointer I/O", QVariant(methodUser));
 
-               if (reqbufs_mmap_cap(reqbuf))
+               if (reqbufs_mmap_cap(reqbuf, 1))
                        m_capMethods->addItem("Memory mapped I/O", QVariant(methodMmap));
        }
        if (m_querycap.capabilities & V4L2_CAP_READWRITE) {

diff --git a/utils/qv4l2-qt4/qv4l2.cpp b/utils/qv4l2-qt4/qv4l2.cpp
index eccdae4..96cb4d7 100644 (file)
--- a/utils/qv4l2-qt4/qv4l2.cpp
+++ b/utils/qv4l2-qt4/qv4l2.cpp
@@ -186,6 +186,7 @@ void ApplicationWindow::capFrame()
        case methodMmap:
                if (!dqbuf_mmap_cap(buf)) {
                        error("dqbuf");
+                       m_capStartAct->setChecked(false);
                        return;
                }
 
@@ -199,6 +200,7 @@ void ApplicationWindow::capFrame()
        case methodUser:
                if (!dqbuf_user_cap(buf)) {
                        error("dqbuf");
+                       m_capStartAct->setChecked(false);
                        return;
                }
 
@@ -217,7 +219,7 @@ void ApplicationWindow::capFrame()
        m_capture->setImage(*m_capImage);
 }
 
-void ApplicationWindow::startCapture(unsigned buffer_size)
+bool ApplicationWindow::startCapture(unsigned buffer_size)
 {
        unsigned int i;
        v4l2_requestbuffers req;
@@ -227,7 +229,7 @@ void ApplicationWindow::startCapture(unsigned buffer_size)
        switch (m_capMethod) {
        case methodRead:
                /* Nothing to do. */
-               return;
+               return true;
 
        case methodMmap:
                if (!reqbufs_mmap_cap(req, 3)) {
@@ -258,27 +260,43 @@ void ApplicationWindow::startCapture(unsigned buffer_size)
                        buf.memory      = V4L2_MEMORY_MMAP;
                        buf.index       = m_nbuffers;
 
-                       if (-1 == ioctl(VIDIOC_QUERYBUF, &buf))
+                       if (-1 == ioctl(VIDIOC_QUERYBUF, &buf)) {
                                perror("VIDIOC_QUERYBUF");
+                               goto error;
+                       }
 
                        m_buffers[m_nbuffers].length = buf.length;
                        m_buffers[m_nbuffers].start = mmap(buf.length, buf.m.offset);
 
-                       if (MAP_FAILED == m_buffers[m_nbuffers].start)
+                       if (MAP_FAILED == m_buffers[m_nbuffers].start) {
                                perror("mmap");
+                               goto error;
+                       }
                }
-               for (i = 0; i < m_nbuffers; ++i)
-                       qbuf_mmap_cap(i);
-               if (!streamon_cap())
+               for (i = 0; i < m_nbuffers; ++i) {
+                       if (!qbuf_mmap_cap(i)) {
+                               perror("VIDIOC_QBUF");
+                               goto error;
+                       }
+               }
+               if (!streamon_cap()) {
                        perror("VIDIOC_STREAMON");
-               return;
+                       goto error;
+               }
+               return true;
 
        case methodUser:
-               if (!reqbufs_user_cap(req)) {
+               if (!reqbufs_user_cap(req, 4)) {
                        error("Cannot capture");
                        break;
                }
 
+               if (req.count < 4) {
+                       error("Too few buffers");
+                       reqbufs_user_cap(req);
+                       break;
+               }
+
                m_buffers = (buffer *)calloc(4, sizeof(*m_buffers));
 
                if (!m_buffers) {
@@ -292,17 +310,24 @@ void ApplicationWindow::startCapture(unsigned buffer_size)
 
                        if (!m_buffers[m_nbuffers].start) {
                                error("Out of memory");
-                               break;
+                               goto error;
                        }
                }
                for (i = 0; i < m_nbuffers; ++i)
-                       qbuf_user_cap(m_buffers[i].start, m_buffers[i].length);
-               if (!streamon_cap())
+                       if (!qbuf_user_cap(i, m_buffers[i].start, m_buffers[i].length)) {
+                               perror("VIDIOC_QBUF");
+                               goto error;
+                       }
+               if (!streamon_cap()) {
                        perror("VIDIOC_STREAMON");
-               return;
+                       goto error;
+               }
+               return true;
        }
 
+error:
        m_capStartAct->setChecked(false);
+       return false;
 }
 
 void ApplicationWindow::stopCapture()
@@ -400,9 +425,10 @@ void ApplicationWindow::capStart(bool start)
        m_capImage->fill(0);
        m_capture->setImage(*m_capImage, true);
        m_capture->show();
-       startCapture(m_capSrcFormat.fmt.pix.sizeimage);
-       m_capNotifier = new QSocketNotifier(fd(), QSocketNotifier::Read, m_tabs);
-       connect(m_capNotifier, SIGNAL(activated(int)), this, SLOT(capFrame()));
+       if (startCapture(m_capSrcFormat.fmt.pix.sizeimage)) {
+               m_capNotifier = new QSocketNotifier(fd(), QSocketNotifier::Read, m_tabs);
+               connect(m_capNotifier, SIGNAL(activated(int)), this, SLOT(capFrame()));
+       }
 }
 
 void ApplicationWindow::closeDevice()

diff --git a/utils/qv4l2-qt4/qv4l2.h b/utils/qv4l2-qt4/qv4l2.h
index 04a4dc9..5c00190 100644 (file)
--- a/utils/qv4l2-qt4/qv4l2.h
+++ b/utils/qv4l2-qt4/qv4l2.h
@@ -81,7 +81,7 @@ public:
 private:
        CaptureWin *m_capture;
 
-       void startCapture(unsigned buffer_size);
+       bool startCapture(unsigned buffer_size);
        void stopCapture();
        void startOutput(unsigned buffer_size);
        void stopOutput();

diff --git a/utils/qv4l2-qt4/v4l2-api.cpp b/utils/qv4l2-qt4/v4l2-api.cpp
index aab0497..b47e645 100644 (file)
--- a/utils/qv4l2-qt4/v4l2-api.cpp
+++ b/utils/qv4l2-qt4/v4l2-api.cpp
@@ -347,11 +347,12 @@ bool v4l2::enum_frameintervals(v4l2_frmivalenum &frm, __u32 init_pixfmt, __u32 w
        return ioctl(VIDIOC_ENUM_FRAMEINTERVALS, &frm) >= 0;
 }
 
-bool v4l2::reqbufs_user_cap(v4l2_requestbuffers &reqbuf)
+bool v4l2::reqbufs_user_cap(v4l2_requestbuffers &reqbuf, int count)
 {
        memset(&reqbuf, 0, sizeof (reqbuf));
        reqbuf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
        reqbuf.memory = V4L2_MEMORY_USERPTR;
+       reqbuf.count = count;
 
        return ioctl(VIDIOC_REQBUFS, &reqbuf) >= 0;
 }
@@ -369,7 +370,6 @@ bool v4l2::reqbufs_mmap_cap(v4l2_requestbuffers &reqbuf, int count)
 bool v4l2::dqbuf_mmap_cap(v4l2_buffer &buf)
 {
        memset(&buf, 0, sizeof(buf));
-
        buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
        buf.memory = V4L2_MEMORY_MMAP;
        return ioctl(VIDIOC_DQBUF, &buf) >= 0;
@@ -378,7 +378,6 @@ bool v4l2::dqbuf_mmap_cap(v4l2_buffer &buf)
 bool v4l2::dqbuf_user_cap(v4l2_buffer &buf)
 {
        memset(&buf, 0, sizeof(buf));
-
        buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
        buf.memory = V4L2_MEMORY_USERPTR;
        return ioctl(VIDIOC_DQBUF, &buf) >= 0;
@@ -393,20 +392,23 @@ bool v4l2::qbuf_mmap_cap(int index)
 {
        v4l2_buffer buf;
 
+       memset(&buf, 0, sizeof(buf));
        buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
        buf.memory = V4L2_MEMORY_MMAP;
        buf.index = index;
        return qbuf(buf);
 }
 
-bool v4l2::qbuf_user_cap(void *ptr, int length)
+bool v4l2::qbuf_user_cap(int index, void *ptr, int length)
 {
        v4l2_buffer buf;
 
+       memset(&buf, 0, sizeof(buf));
        buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
        buf.memory = V4L2_MEMORY_USERPTR;
        buf.m.userptr = (unsigned long)ptr;
        buf.length = length;
+       buf.index = index;
        return qbuf(buf);
 }
 
@@ -446,7 +448,6 @@ bool v4l2::reqbufs_mmap_out(v4l2_requestbuffers &reqbuf, int count)
 bool v4l2::dqbuf_mmap_out(v4l2_buffer &buf)
 {
        memset(&buf, 0, sizeof(buf));
-
        buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
        buf.memory = V4L2_MEMORY_MMAP;
        return ioctl("dqbuf", VIDIOC_DQBUF, &buf);
@@ -455,7 +456,6 @@ bool v4l2::dqbuf_mmap_out(v4l2_buffer &buf)
 bool v4l2::dqbuf_user_out(v4l2_buffer &buf)
 {
        memset(&buf, 0, sizeof(buf));
-
        buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
        buf.memory = V4L2_MEMORY_USERPTR;
        return ioctl(VIDIOC_DQBUF, &buf) >= 0;

diff --git a/utils/qv4l2-qt4/v4l2-api.h b/utils/qv4l2-qt4/v4l2-api.h
index 0676256..99c67b6 100644 (file)
--- a/utils/qv4l2-qt4/v4l2-api.h
+++ b/utils/qv4l2-qt4/v4l2-api.h
@@ -83,12 +83,12 @@ public:
        bool enum_frameintervals(v4l2_frmivalenum &frm, __u32 init_pixfmt = 0, __u32 w = 0, __u32 h = 0, int index = 0);
 
        bool reqbufs_mmap_cap(v4l2_requestbuffers &reqbuf, int count = 0);
-       bool reqbufs_user_cap(v4l2_requestbuffers &reqbuf);
+       bool reqbufs_user_cap(v4l2_requestbuffers &reqbuf, int count = 0);
        bool dqbuf_mmap_cap(v4l2_buffer &buf);
        bool dqbuf_user_cap(v4l2_buffer &buf);
        bool qbuf(v4l2_buffer &buf);
        bool qbuf_mmap_cap(int index);
-       bool qbuf_user_cap(void *ptr, int length);
+       bool qbuf_user_cap(int index, void *ptr, int length);
        bool streamon_cap();
        bool streamoff_cap();