v4l2: fix and cleanup of mmap buffers allocation

Especially fix memory leaks on error.

modules/access/v4l2/access.c

@@ -96,7 +96,6 @@ error:
 int InitVideo (access_t *access, int fd)
 {
     demux_sys_t *sys = (demux_sys_t *)access->p_sys;
-    enum v4l2_buf_type buf_type;
 
     /* Get device capabilites */
     struct v4l2_capability cap;
@@ -168,9 +167,10 @@ int InitVideo (access_t *access, int fd)
     /* Init I/O method */
     if (cap.capabilities & V4L2_CAP_STREAMING)
     {
-        if (InitMmap (VLC_OBJECT(access), sys, fd))
+        sys->bufv = InitMmap (VLC_OBJECT(access), fd, &sys->bufc);
+        if (sys->bufv == NULL)
             return -1;
-        for (unsigned int i = 0; i < sys->i_nbuffers; i++)
+        for (uint32_t i = 0; i < sys->bufc; i++)
         {
             struct v4l2_buffer buf = {
                 .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
@@ -185,7 +185,7 @@ int InitVideo (access_t *access, int fd)
             }
         }
 
-        buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+        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" );

modules/access/v4l2/demux.c

@@ -430,9 +430,10 @@ static int InitVideo (demux_t *demux, int fd)
     void *(*entry) (void *);
     if (caps & V4L2_CAP_STREAMING)
     {
-        if (InitMmap (VLC_OBJECT(demux), sys, fd))
+        sys->bufv = InitMmap (VLC_OBJECT(demux), fd, &sys->bufc);
+        if (sys->bufv == NULL)
             return -1;
-        for (unsigned int i = 0; i < sys->i_nbuffers; i++)
+        for (uint32_t i = 0; i < sys->bufc; i++)
         {
             struct v4l2_buffer buf = {
                 .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
@@ -494,37 +495,22 @@ void DemuxClose( vlc_object_t *obj )
         {
             /* NOTE: Some buggy drivers hang if buffers are not unmapped before
              * streamoff */
-            for( unsigned i = 0; i < sys->i_nbuffers; i++ )
+            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_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( sys->i_fd, VIDIOC_STREAMOFF, &buf_type );
+            free (sys->bufv);
             break;
         }
     }
 
-    /* Free Video Buffers */
-    if( sys->p_buffers ) {
-        switch( sys->io )
-        {
-        case IO_METHOD_READ:
-            free( sys->p_buffers[0].start );
-            break;
-
-        case IO_METHOD_MMAP:
-            for( unsigned i = 0; i < sys->i_nbuffers; ++i )
-                v4l2_munmap( sys->p_buffers[i].start,
-                             sys->p_buffers[i].length );
-            break;
-        }
-        free( sys->p_buffers );
-    }
-
     ControlsDeinit( obj, sys->controls );
     v4l2_close( fd );
     free( sys );

modules/access/v4l2/v4l2.h

@@ -86,9 +86,9 @@ struct demux_sys_t
     /* Video */
     io_method io;
 
-    struct buffer_t *p_buffers;
-    unsigned int i_nbuffers;
-#define blocksize i_nbuffers /* HACK HACK */
+    struct buffer_t *bufv;
+    uint32_t bufc;
+#define blocksize bufc /* HACK HACK */
 
     uint32_t i_block_flags;
 
@@ -110,8 +110,7 @@ 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)
-
-int InitMmap (vlc_object_t *, demux_sys_t *, int);
+struct buffer_t *InitMmap (vlc_object_t *, int, uint32_t *);
 block_t* GrabVideo(vlc_object_t *, demux_sys_t *);
 
 /* demux.c */

modules/access/v4l2/video.c

@@ -1001,7 +1001,7 @@ block_t* GrabVideo (vlc_object_t *demux, demux_sys_t *sys)
         }
     }
 
-    if (buf.index >= sys->i_nbuffers) {
+    if (buf.index >= sys->bufc) {
         msg_Err (demux, "Failed capturing new frame as i>=nbuffers");
         return NULL;
     }
@@ -1010,7 +1010,7 @@ block_t* GrabVideo (vlc_object_t *demux, demux_sys_t *sys)
     block_t *block = block_Alloc (buf.bytesused);
     if (unlikely(block == NULL))
         return NULL;
-    memcpy (block->p_buffer, sys->p_buffers[buf.index].start, buf.bytesused);
+    memcpy (block->p_buffer, sys->bufv[buf.index].start, buf.bytesused);
 
     /* Unlock */
     if (v4l2_ioctl (sys->i_fd, VIDIOC_QBUF, &buf) < 0)
@@ -1025,56 +1025,63 @@ block_t* GrabVideo (vlc_object_t *demux, demux_sys_t *sys)
 /*****************************************************************************
  * Helper function to initalise video IO using the mmap method
  *****************************************************************************/
-int InitMmap( vlc_object_t *p_demux, demux_sys_t *p_sys, int i_fd )
+struct buffer_t *InitMmap (vlc_object_t *obj, int fd, uint32_t *restrict n)
 {
-    struct v4l2_requestbuffers req;
-
-    memset( &req, 0, sizeof(req) );
-    req.count = 4;
-    req.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-    req.memory = V4L2_MEMORY_MMAP;
+    struct v4l2_requestbuffers req = {
+        .count = 4,
+        .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+        .memory = V4L2_MEMORY_MMAP,
+    };
 
-    if( v4l2_ioctl( i_fd, VIDIOC_REQBUFS, &req ) < 0 )
+    if (v4l2_ioctl (fd, VIDIOC_REQBUFS, &req) < 0)
     {
-        msg_Err( p_demux, "device does not support mmap I/O" );
-        return -1;
+        msg_Err (obj, "cannot allocate buffers: %m" );
+        return NULL;
     }
 
-    if( req.count < 2 )
+    if (req.count < 2)
     {
-        msg_Err( p_demux, "insufficient buffers" );
-        return -1;
+        msg_Err (obj, "cannot allocate enough buffers");
+        return NULL;
     }
 
-    p_sys->p_buffers = calloc( req.count, sizeof( *p_sys->p_buffers ) );
-    if( unlikely(!p_sys->p_buffers) )
-        return -1;
+    struct buffer_t *bufv = malloc (req.count * sizeof (*bufv));
+    if (unlikely(bufv == NULL))
+        return NULL;
 
-    for( p_sys->i_nbuffers = 0; p_sys->i_nbuffers < req.count; ++p_sys->i_nbuffers )
+    uint32_t bufc;
+    for (bufc = 0; bufc < req.count; bufc++)
     {
-        struct v4l2_buffer buf;
-
-        memset( &buf, 0, sizeof(buf) );
-        buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-        buf.memory = V4L2_MEMORY_MMAP;
-        buf.index = p_sys->i_nbuffers;
+        struct v4l2_buffer buf = {
+            .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+            .memory = V4L2_MEMORY_MMAP,
+            .index = bufc,
+        };
 
-        if( v4l2_ioctl( i_fd, VIDIOC_QUERYBUF, &buf ) < 0 )
+        if (v4l2_ioctl (fd, VIDIOC_QUERYBUF, &buf) < 0)
         {
-            msg_Err( p_demux, "VIDIOC_QUERYBUF: %m" );
-            return -1;
+            msg_Err (obj, "cannot query buffer %"PRIu32": %m", bufc);
+            goto error;
         }
 
-        p_sys->p_buffers[p_sys->i_nbuffers].length = buf.length;
-        p_sys->p_buffers[p_sys->i_nbuffers].start =
-            v4l2_mmap( NULL, buf.length, PROT_READ | PROT_WRITE, MAP_SHARED, i_fd, buf.m.offset );
-
-        if( p_sys->p_buffers[p_sys->i_nbuffers].start == MAP_FAILED )
+        bufv[bufc].start = v4l2_mmap (NULL, buf.length, PROT_READ | PROT_WRITE,
+                                      MAP_SHARED, fd, buf.m.offset);
+        if (bufv[bufc].start == MAP_FAILED)
         {
-            msg_Err( p_demux, "mmap failed: %m" );
-            return -1;
+            msg_Err (obj, "cannot map buffer %"PRIu32": %m", bufc);
+            goto error;
         }
+        bufv[bufc].length = buf.length;
     }
 
-    return 0;
+    *n = bufc;
+    return bufv;
+error:
+    while (bufc > 0)
+    {
+        bufc--;
+        v4l2_munmap (bufv[bufc].start, bufv[bufc].length);
+    }
+    free (bufv);
+    return NULL;
 }