v4l2: various cleanups

Various cleanups, avoids useless casts, move error handling outside of the main
code flow.
Negotiate to a resonable resolution instead of the max resolution.

sys/v4l2/gstv4l2bufferpool.c

@@ -34,9 +34,7 @@
 
 #include <gstv4l2bufferpool.h>
 #include "gstv4l2src.h"
-#ifdef HAVE_EXPERIMENTAL
 #include "gstv4l2sink.h"
-#endif
 #include "v4l2_calls.h"
 #include "gst/gst-i18n-plugin.h"
 
@@ -194,12 +192,11 @@ mmap_failed:
   }
 }
 
-
 /*
  * GstV4l2BufferPool:
  */
-
+#define gst_v4l2_buffer_pool_parent_class parent_class
-static GObjectClass *buffer_pool_parent_class = NULL;
+G_DEFINE_TYPE (GstV4l2BufferPool, gst_v4l2_buffer_pool, G_TYPE_OBJECT);
 
 static void
 gst_v4l2_buffer_pool_finalize (GObject * object)
@@ -220,11 +217,11 @@ gst_v4l2_buffer_pool_finalize (GObject * object)
     pool->buffers = NULL;
   }
 
-  buffer_pool_parent_class->finalize (object);
+  G_OBJECT_CLASS (parent_class)->finalize (object);
 }
 
 static void
-gst_v4l2_buffer_pool_init (GstV4l2BufferPool * pool, gpointer g_class)
+gst_v4l2_buffer_pool_init (GstV4l2BufferPool * pool)
 {
   pool->lock = g_mutex_new ();
   pool->running = FALSE;
@@ -232,40 +229,13 @@ gst_v4l2_buffer_pool_init (GstV4l2BufferPool * pool, gpointer g_class)
 }
 
 static void
-gst_v4l2_buffer_pool_class_init (gpointer g_class, gpointer class_data)
+gst_v4l2_buffer_pool_class_init (GstV4l2BufferPoolClass * klass)
 {
-  GObjectClass *object_class = G_OBJECT_CLASS (g_class);
+  GObjectClass *object_class = G_OBJECT_CLASS (klass);
-
-  buffer_pool_parent_class = g_type_class_peek_parent (g_class);
 
   object_class->finalize = gst_v4l2_buffer_pool_finalize;
 }
 
-GType
-gst_v4l2_buffer_pool_get_type (void)
-{
-  static GType _gst_v4l2_buffer_pool_type;
-
-  if (G_UNLIKELY (_gst_v4l2_buffer_pool_type == 0)) {
-    static const GTypeInfo v4l2_buffer_pool_info = {
-      sizeof (GObjectClass),
-      NULL,
-      NULL,
-      gst_v4l2_buffer_pool_class_init,
-      NULL,
-      NULL,
-      sizeof (GstV4l2BufferPool),
-      0,
-      (GInstanceInitFunc) gst_v4l2_buffer_pool_init,
-      NULL
-    };
-    _gst_v4l2_buffer_pool_type = g_type_register_static (G_TYPE_OBJECT,
-        "GstV4l2BufferPool", &v4l2_buffer_pool_info, 0);
-  }
-  return _gst_v4l2_buffer_pool_type;
-}
-
-
 /* this is somewhat of a hack.. but better to keep the hack in
  * one place than copy/pasting it around..
  */
@@ -275,18 +245,14 @@ get_v4l2_object (GstElement * v4l2elem)
   GstV4l2Object *v4l2object = NULL;
   if (GST_IS_V4L2SRC (v4l2elem)) {
     v4l2object = (GST_V4L2SRC (v4l2elem))->v4l2object;
-#ifdef HAVE_EXPERIMENTAL
   } else if (GST_IS_V4L2SINK (v4l2elem)) {
     v4l2object = (GST_V4L2SINK (v4l2elem))->v4l2object;
-#endif
   } else {
     GST_ERROR_OBJECT (v4l2elem, "unknown v4l2 element");
   }
   return v4l2object;
 }
 
-
-
 /**
  * gst_v4l2_buffer_pool_new:
  * @v4l2elem:  the v4l2 element (src or sink) that owns this pool
@@ -316,7 +282,6 @@ gst_v4l2_buffer_pool_new (GstElement * v4l2elem, gint fd, gint num_buffers,
   if (pool->video_fd < 0)
     goto dup_failed;
 
-
   /* first, lets request buffers, and see how many we can get: */
   GST_DEBUG_OBJECT (v4l2elem, "STREAMING, requesting %d MMAP buffers",
       num_buffers);
@@ -428,7 +393,7 @@ gst_v4l2_buffer_pool_destroy (GstV4l2BufferPool * pool)
     GstBuffer *buf;
 
     GST_V4L2_BUFFER_POOL_LOCK (pool);
-    buf = GST_BUFFER (pool->buffers[n]);
+    buf = pool->buffers[n];
     GST_V4L2_BUFFER_POOL_UNLOCK (pool);
 
     if (buf)
@@ -493,13 +458,20 @@ gst_v4l2_buffer_pool_qbuf (GstV4l2BufferPool * pool, GstBuffer * buf)
       meta->vbuffer.index);
 
   if (v4l2_ioctl (pool->video_fd, VIDIOC_QBUF, &meta->vbuffer) < 0)
-    return FALSE;
+    goto queue_failed;
 
   pool->num_live_buffers--;
   GST_DEBUG_OBJECT (pool->v4l2elem, "num_live_buffers--: %d",
       pool->num_live_buffers);
 
   return TRUE;
+
+  /* ERRORS */
+queue_failed:
+  {
+    GST_WARNING_OBJECT (pool->v4l2elem, "could not queue a buffer");
+    return FALSE;
+  }
 }
 
 /**
@@ -523,7 +495,8 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool)
   buffer.type = pool->type;
   buffer.memory = V4L2_MEMORY_MMAP;
 
-  if (v4l2_ioctl (pool->video_fd, VIDIOC_DQBUF, &buffer) >= 0) {
+  if (v4l2_ioctl (pool->video_fd, VIDIOC_DQBUF, &buffer) < 0)
+    goto error;
 
   GST_V4L2_BUFFER_POOL_LOCK (pool);
 
@@ -532,14 +505,8 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool)
    */
   pool_buffer = pool->buffers[buffer.index];
 
-    if (pool_buffer == NULL) {
+  if (pool_buffer == NULL)
-      GST_ELEMENT_ERROR (pool->v4l2elem, RESOURCE, FAILED,
+    goto no_buffers;
-          (_("Failed trying to get video frames from device '%s'."),
-              v4l2object->videodev),
-          (_("No free buffers found in the pool at index %d."), buffer.index));
-      GST_V4L2_BUFFER_POOL_UNLOCK (pool);
-      return NULL;
-    }
 
   GST_LOG_OBJECT (pool->v4l2elem,
       "grabbed frame %d (ix=%d), flags %08x, pool-ct=%d, buffer=%p",
@@ -562,9 +529,10 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool)
   GST_V4L2_BUFFER_POOL_UNLOCK (pool);
 
   return pool_buffer;
-  }
-
 
+  /* ERRORS */
+error:
+  {
     GST_WARNING_OBJECT (pool->v4l2elem,
         "problem grabbing frame %d (ix=%d), pool-ct=%d, buf.flags=%d",
         buffer.sequence, buffer.index,
@@ -581,9 +549,7 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool)
         GST_ELEMENT_ERROR (pool->v4l2elem, RESOURCE, FAILED,
             (_("Failed trying to get video frames from device '%s'."),
                 v4l2object->videodev),
-          (_("The buffer type is not supported, or the index is out of bounds,"
+            (_("The buffer type is not supported, or the index is out of bounds," " or no buffers have been allocated yet, or the userptr" " or length are invalid. device %s"), v4l2object->videodev));
-                  " or no buffers have been allocated yet, or the userptr"
-                  " or length are invalid. device %s"), v4l2object->videodev));
         break;
       case ENOMEM:
         GST_ELEMENT_ERROR (pool->v4l2elem, RESOURCE, FAILED,
@@ -612,9 +578,18 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool)
             v4l2object->videodev, errno, g_strerror (errno));
         break;
     }
-
     return NULL;
   }
+no_buffers:
+  {
+    GST_ELEMENT_ERROR (pool->v4l2elem, RESOURCE, FAILED,
+        (_("Failed trying to get video frames from device '%s'."),
+            v4l2object->videodev),
+        (_("No free buffers found in the pool at index %d."), buffer.index));
+    GST_V4L2_BUFFER_POOL_UNLOCK (pool);
+    return NULL;
+  }
+}
 
 /**
  * gst_v4l2_buffer_pool_available_buffers:

sys/v4l2/gstv4l2bufferpool.h

@@ -39,6 +39,7 @@ GType gst_v4l2_buffer_pool_get_type (void);
 #define GST_V4L2_BUFFER_POOL(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GST_TYPE_V4L2_BUFFER_POOL, GstV4l2BufferPool))
 
 typedef struct _GstV4l2BufferPool GstV4l2BufferPool;
+typedef struct _GstV4l2BufferPoolClass GstV4l2BufferPoolClass;
 typedef struct _GstMetaV4l2 GstMetaV4l2;
 
 
@@ -59,6 +60,11 @@ struct _GstV4l2BufferPool
   GstBuffer **buffers;
 };
 
+struct _GstV4l2BufferPoolClass
+{
+  GObjectClass parent_class;
+};
+
 struct _GstMetaV4l2 {
   GstMeta meta;
 

sys/v4l2/gstv4l2sink.c

@@ -733,7 +733,7 @@ gst_v4l2sink_buffer_alloc (GstBaseSink * bsink, guint64 offset, guint size,
 
     if (G_LIKELY (v4l2buf)) {
       GST_DEBUG_OBJECT (v4l2sink, "allocated buffer: %p", v4l2buf);
-      *buf = GST_BUFFER (v4l2buf);
+      *buf = v4l2buf;
       return GST_FLOW_OK;
     } else {
       GST_DEBUG_OBJECT (v4l2sink, "failed to allocate buffer");
@@ -834,7 +834,7 @@ gst_v4l2sink_show_frame (GstBaseSink * bsink, GstBuffer * buf)
      * to the pool of available buffers..  and if not, we keep looping.
      */
     if (v4l2buf) {
-      gst_buffer_unref (GST_BUFFER (v4l2buf));
+      gst_buffer_unref (v4l2buf);
     }
   }
 

sys/v4l2/gstv4l2src.c

@@ -277,7 +277,6 @@ gst_v4l2src_set_property (GObject * object,
   }
 }
 
-
 static void
 gst_v4l2src_get_property (GObject * object,
     guint prop_id, GValue * value, GParamSpec * pspec)
@@ -303,7 +302,6 @@ gst_v4l2src_get_property (GObject * object,
   }
 }
 
-
 /* this function is a bit of a last resort */
 static void
 gst_v4l2src_fixate (GstBaseSrc * basesrc, GstCaps * caps)
@@ -318,14 +316,10 @@ gst_v4l2src_fixate (GstBaseSrc * basesrc, GstCaps * caps)
 
     structure = gst_caps_get_structure (caps, i);
 
-    /* FIXME such sizes? we usually fixate to something in the 320x200
+    /* We are fixating to a resonable 320x200 resolution
-     * range... */
-    /* We are fixating to greater possble size (limited to GST_V4L2_MAX_SIZE)
        and the maximum framerate resolution for that size */
-    gst_structure_fixate_field_nearest_int (structure, "width",
+    gst_structure_fixate_field_nearest_int (structure, "width", 320);
-        GST_V4L2_MAX_SIZE);
+    gst_structure_fixate_field_nearest_int (structure, "height", 200);
-    gst_structure_fixate_field_nearest_int (structure, "height",
-        GST_V4L2_MAX_SIZE);
     gst_structure_fixate_field_nearest_fraction (structure, "framerate",
         G_MAXINT, 1);
 
@@ -394,11 +388,8 @@ gst_v4l2src_negotiate (GstBaseSrc * basesrc)
        * resolution strictly bigger then the first peer caps */
       if (gst_caps_get_size (icaps) > 1) {
         GstStructure *s = gst_caps_get_structure (peercaps, 0);
-
         int best = 0;
-
         int twidth, theight;
-
         int width = G_MAXINT, height = G_MAXINT;
 
         if (gst_structure_get_int (s, "width", &twidth)
@@ -409,7 +400,6 @@ gst_v4l2src_negotiate (GstBaseSrc * basesrc)
            */
           for (i = gst_caps_get_size (icaps) - 1; i >= 0; i--) {
             GstStructure *is = gst_caps_get_structure (icaps, i);
-
             int w, h;
 
             if (gst_structure_get_int (is, "width", &w)
@@ -493,7 +483,6 @@ gst_v4l2src_get_caps (GstBaseSrc * src, GstCaps * filter)
 
   for (walk = formats; walk; walk = walk->next) {
     struct v4l2_fmtdesc *format;
-
     GstStructure *template;
 
     format = (struct v4l2_fmtdesc *) walk->data;
@@ -868,6 +857,7 @@ gst_v4l2src_create (GstPushSrc * src, GstBuffer ** buf)
   if (v4l2src->get_frame == NULL)
     goto not_negotiated;
 
+  /* decimate, just capture and throw away frames */
   for (i = 0; i < v4l2src->decimate - 1; i++) {
     ret = v4l2src->get_frame (v4l2src, buf);
     if (ret != GST_FLOW_OK) {

sys/v4l2/v4l2src_calls.c

@@ -127,7 +127,7 @@ gst_v4l2src_grab_frame (GstV4l2Src * v4l2src, GstBuffer ** buf)
       }
     }
 
-    pool_buffer = GST_BUFFER (gst_v4l2_buffer_pool_dqbuf (pool));
+    pool_buffer = gst_v4l2_buffer_pool_dqbuf (pool);
     if (pool_buffer)
       break;