[vlc-commits] v4l2: factorize and simplify mmap() initialization

Rémi Denis-Courmont git at videolan.org
Wed Apr 11 20:01:50 CEST 2012


vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Wed Apr 11 20:49:24 2012 +0300| [8d9f4bb6bdec56b957577812b00f7810888b22f4] | committer: Rémi Denis-Courmont

v4l2: factorize and simplify mmap() initialization

Also fix huge memory leak in v4l2 access (buffers never released).

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=8d9f4bb6bdec56b957577812b00f7810888b22f4
---

 modules/access/v4l2/access.c |   28 ++++--------------------
 modules/access/v4l2/demux.c  |   47 +++--------------------------------------
 modules/access/v4l2/v4l2.h   |    5 +++-
 modules/access/v4l2/video.c  |   41 ++++++++++++++++++++++++++---------
 4 files changed, 43 insertions(+), 78 deletions(-)

diff --git a/modules/access/v4l2/access.c b/modules/access/v4l2/access.c
index 05e0ab4..3cd0e56 100644
--- a/modules/access/v4l2/access.c
+++ b/modules/access/v4l2/access.c
@@ -180,36 +180,16 @@ int InitVideo (access_t *access, int fd)
     /* Init I/O method */
     if (cap.capabilities & V4L2_CAP_STREAMING)
     {
-        sys->bufv = InitMmap (VLC_OBJECT(access), fd, &sys->bufc);
+        sys->bufc = 4;
+        sys->bufv = StartMmap (VLC_OBJECT(access), fd, &sys->bufc);
         if (sys->bufv == NULL)
             return -1;
-        for (uint32_t i = 0; i < sys->bufc; i++)
-        {
-            struct v4l2_buffer buf = {
-                .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
-                .memory = V4L2_MEMORY_MMAP,
-                .index = i,
-            };
-
-            if (v4l2_ioctl (fd, VIDIOC_QBUF, &buf) < 0)
-            {
-                msg_Err (access, "cannot queue buffer: %m");
-                return -1;
-            }
-        }
-
-        enum v4l2_buf_type buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-        if (v4l2_ioctl (fd, VIDIOC_STREAMON, &buf_type) < 0)
-        {
-            msg_Err (access, "cannot start streaming: %m" );
-            return -1;
-        }
-
         access->pf_block = AccessRead;
     }
     else if (cap.capabilities & V4L2_CAP_READWRITE)
     {
         sys->blocksize = fmt.fmt.pix.sizeimage;
+        sys->bufv = NULL;
         access->pf_read = AccessReadStream;
     }
     else
@@ -225,6 +205,8 @@ void AccessClose( vlc_object_t *obj )
     access_t *access = (access_t *)obj;
     access_sys_t *sys = access->p_sys;
 
+    if (sys->bufv != NULL)
+        StopMmap (sys->fd, sys->bufv, sys->bufc);
     ControlsDeinit( obj, sys->controls );
     v4l2_close (sys->fd);
     free( sys );
diff --git a/modules/access/v4l2/demux.c b/modules/access/v4l2/demux.c
index b9b946d..7a43ba9 100644
--- a/modules/access/v4l2/demux.c
+++ b/modules/access/v4l2/demux.c
@@ -438,30 +438,10 @@ static int InitVideo (demux_t *demux, int fd)
     void *(*entry) (void *);
     if (caps & V4L2_CAP_STREAMING)
     {
-        sys->bufv = InitMmap (VLC_OBJECT(demux), fd, &sys->bufc);
+        sys->bufc = 4;
+        sys->bufv = StartMmap (VLC_OBJECT(demux), fd, &sys->bufc);
         if (sys->bufv == NULL)
             return -1;
-        for (uint32_t i = 0; i < sys->bufc; i++)
-        {
-            struct v4l2_buffer buf = {
-                .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
-                .memory = V4L2_MEMORY_MMAP,
-                .index = i,
-            };
-
-            if (v4l2_ioctl (fd, VIDIOC_QBUF, &buf) < 0)
-            {
-                msg_Err (demux, "cannot queue buffer: %m");
-                return -1;
-            }
-        }
-
-        enum v4l2_buf_type buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-        if (v4l2_ioctl (fd, VIDIOC_STREAMON, &buf_type) < 0)
-        {
-            msg_Err (demux, "cannot start streaming: %m");
-            return -1;
-        }
         entry = StreamThread;
     }
     else if (caps & V4L2_CAP_READWRITE)
@@ -485,32 +465,13 @@ void DemuxClose( vlc_object_t *obj )
 {
     demux_t *demux = (demux_t *)obj;
     demux_sys_t *sys = demux->p_sys;
-    int fd = sys->fd;
 
     vlc_cancel (sys->thread);
     vlc_join (sys->thread, NULL);
-
-    /* Stop video capture */
     if (sys->bufv != NULL)
-    {
-        for (uint32_t i = 0; i < sys->bufc; i++)
-        {
-            struct v4l2_buffer buf = {
-                .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
-                .memory = V4L2_MEMORY_MMAP,
-            };
-
-            v4l2_ioctl (fd, VIDIOC_DQBUF, &buf);
-            v4l2_munmap (sys->bufv[i].start, sys->bufv[i].length);
-        }
-
-        enum v4l2_buf_type buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-        v4l2_ioctl (fd, VIDIOC_STREAMOFF, &buf_type);
-        free (sys->bufv);
-    }
-
+        StopMmap (sys->fd, sys->bufv, sys->bufc);
     ControlsDeinit( obj, sys->controls );
-    v4l2_close( fd );
+    v4l2_close (sys->fd);
     free( sys );
 }
 
diff --git a/modules/access/v4l2/v4l2.h b/modules/access/v4l2/v4l2.h
index 1ee1d00..459b312 100644
--- a/modules/access/v4l2/v4l2.h
+++ b/modules/access/v4l2/v4l2.h
@@ -87,7 +87,10 @@ int SetupFormat (vlc_object_t *, int, uint32_t,
                  struct v4l2_format *, struct v4l2_streamparm *);
 #define SetupFormat(o,fd,fcc,fmt,p) \
         SetupFormat(VLC_OBJECT(o),fd,fcc,fmt,p)
-struct buffer_t *InitMmap (vlc_object_t *, int, uint32_t *);
+
+struct buffer_t *StartMmap (vlc_object_t *, int, uint32_t *);
+void StopMmap (int, struct buffer_t *, uint32_t);
+
 block_t* GrabVideo (vlc_object_t *, int, const struct buffer_t *);
 
 /* demux.c */
diff --git a/modules/access/v4l2/video.c b/modules/access/v4l2/video.c
index 508d9be..2ed87c6 100644
--- a/modules/access/v4l2/video.c
+++ b/modules/access/v4l2/video.c
@@ -556,13 +556,15 @@ block_t *GrabVideo (vlc_object_t *demux, int fd,
     return block;
 }
 
-/*****************************************************************************
- * Helper function to initalise video IO using the mmap method
- *****************************************************************************/
-struct buffer_t *InitMmap (vlc_object_t *obj, int fd, uint32_t *restrict n)
+/**
+ * Allocates memory-mapped buffers, queues them and start streaming.
+ * @param n requested buffers count [IN], allocated buffers count [OUT]
+ * @return array of allocated buffers (use free()), or NULL on error.
+ */
+struct buffer_t *StartMmap (vlc_object_t *obj, int fd, uint32_t *restrict n)
 {
     struct v4l2_requestbuffers req = {
-        .count = 4,
+        .count = *n,
         .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
         .memory = V4L2_MEMORY_MMAP,
     };
@@ -597,6 +599,11 @@ struct buffer_t *InitMmap (vlc_object_t *obj, int fd, uint32_t *restrict n)
             msg_Err (obj, "cannot query buffer %"PRIu32": %m", bufc);
             goto error;
         }
+        if (v4l2_ioctl (fd, VIDIOC_QBUF, &buf) < 0)
+        {
+            msg_Err (obj, "cannot queue buffer %"PRIu32": %m", bufc);
+            goto error;
+        }
 
         bufv[bufc].start = v4l2_mmap (NULL, buf.length, PROT_READ | PROT_WRITE,
                                       MAP_SHARED, fd, buf.m.offset);
@@ -608,14 +615,26 @@ struct buffer_t *InitMmap (vlc_object_t *obj, int fd, uint32_t *restrict n)
         bufv[bufc].length = buf.length;
     }
 
+    enum v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+    if (v4l2_ioctl (fd, VIDIOC_STREAMON, &type) < 0)
+    {
+        msg_Err (obj, "cannot start streaming: %m");
+        goto error;
+    }
     *n = bufc;
     return bufv;
 error:
-    while (bufc > 0)
-    {
-        bufc--;
-        v4l2_munmap (bufv[bufc].start, bufv[bufc].length);
-    }
-    free (bufv);
+    StopMmap (fd, bufv, bufc);
     return NULL;
 }
+
+void StopMmap (int fd, struct buffer_t *bufv, uint32_t bufc)
+{
+    enum v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+
+    /* STREAMOFF implicitly dequeues all buffers */
+    v4l2_ioctl (fd, VIDIOC_STREAMOFF, &type);
+    for (uint32_t i = bufc; i < bufc; i++)
+        v4l2_munmap (bufv[i].start, bufv[i].length);
+    free (bufv);
+}



More information about the vlc-commits mailing list