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.

diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c
index adf9cca..1025049 100644 (file)
--- a/sys/v4l2/gstv4l2bufferpool.c
+++ b/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:
  */
-
-static GObjectClass *buffer_pool_parent_class = NULL;
+#define gst_v4l2_buffer_pool_parent_class parent_class
+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);
-
-  buffer_pool_parent_class = g_type_class_peek_parent (g_class);
+  GObjectClass *object_class = G_OBJECT_CLASS (klass);
 
   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,97 +495,100 @@ 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);
+  GST_V4L2_BUFFER_POOL_LOCK (pool);
 
-    /* get our GstBuffer with that index from the pool, if the buffer was
-     * outstanding we have a serious problem.
-     */
-    pool_buffer = pool->buffers[buffer.index];
-
-    if (pool_buffer == NULL) {
-      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;
-    }
+  /* get our GstBuffer with that index from the pool, if the buffer was
+   * outstanding we have a serious problem.
+   */
+  pool_buffer = pool->buffers[buffer.index];
 
-    GST_LOG_OBJECT (pool->v4l2elem,
-        "grabbed frame %d (ix=%d), flags %08x, pool-ct=%d, buffer=%p",
-        buffer.sequence, buffer.index, buffer.flags, pool->num_live_buffers,
-        pool_buffer);
+  if (pool_buffer == NULL)
+    goto no_buffers;
 
-    pool->num_live_buffers++;
-    GST_DEBUG_OBJECT (pool->v4l2elem, "num_live_buffers++: %d",
-        pool->num_live_buffers);
+  GST_LOG_OBJECT (pool->v4l2elem,
+      "grabbed frame %d (ix=%d), flags %08x, pool-ct=%d, buffer=%p",
+      buffer.sequence, buffer.index, buffer.flags, pool->num_live_buffers,
+      pool_buffer);
 
-    /* set top/bottom field first if v4l2_buffer has the information */
-    if (buffer.field == V4L2_FIELD_INTERLACED_TB)
-      GST_BUFFER_FLAG_SET (pool_buffer, GST_VIDEO_BUFFER_TFF);
-    if (buffer.field == V4L2_FIELD_INTERLACED_BT)
-      GST_BUFFER_FLAG_UNSET (pool_buffer, GST_VIDEO_BUFFER_TFF);
+  pool->num_live_buffers++;
+  GST_DEBUG_OBJECT (pool->v4l2elem, "num_live_buffers++: %d",
+      pool->num_live_buffers);
 
-    /* this can change at every frame, esp. with jpeg */
-    gst_buffer_resize (pool_buffer, 0, buffer.bytesused);
+  /* set top/bottom field first if v4l2_buffer has the information */
+  if (buffer.field == V4L2_FIELD_INTERLACED_TB)
+    GST_BUFFER_FLAG_SET (pool_buffer, GST_VIDEO_BUFFER_TFF);
+  if (buffer.field == V4L2_FIELD_INTERLACED_BT)
+    GST_BUFFER_FLAG_UNSET (pool_buffer, GST_VIDEO_BUFFER_TFF);
 
-    GST_V4L2_BUFFER_POOL_UNLOCK (pool);
+  /* this can change at every frame, esp. with jpeg */
+  gst_buffer_resize (pool_buffer, 0, buffer.bytesused);
 
-    return pool_buffer;
-  }
+  GST_V4L2_BUFFER_POOL_UNLOCK (pool);
 
+  return pool_buffer;
 
-  GST_WARNING_OBJECT (pool->v4l2elem,
-      "problem grabbing frame %d (ix=%d), pool-ct=%d, buf.flags=%d",
-      buffer.sequence, buffer.index,
-      GST_MINI_OBJECT_REFCOUNT (pool), buffer.flags);
-
-  switch (errno) {
-    case EAGAIN:
-      GST_WARNING_OBJECT (pool->v4l2elem,
-          "Non-blocking I/O has been selected using O_NONBLOCK and"
-          " no buffer was in the outgoing queue. device %s",
-          v4l2object->videodev);
-      break;
-    case EINVAL:
-      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,"
-                  " 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,
-          (_("Failed trying to get video frames from device '%s'. Not enough memory."), v4l2object->videodev), (_("insufficient memory to enqueue a user pointer buffer. device %s."), v4l2object->videodev));
-      break;
-    case EIO:
-      GST_INFO_OBJECT (pool->v4l2elem,
-          "VIDIOC_DQBUF failed due to an internal error."
-          " Can also indicate temporary problems like signal loss."
-          " Note the driver might dequeue an (empty) buffer despite"
-          " returning an error, or even stop capturing."
-          " device %s", v4l2object->videodev);
-      /* have we de-queued a buffer ? */
-      if (!(buffer.flags & (V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_DONE))) {
-        GST_DEBUG_OBJECT (pool->v4l2elem, "reenqueing buffer");
-        /* FIXME ... should we do something here? */
-      }
-      break;
-    case EINTR:
-      GST_WARNING_OBJECT (pool->v4l2elem,
-          "could not sync on a buffer on device %s", v4l2object->videodev);
-      break;
-    default:
-      GST_WARNING_OBJECT (pool->v4l2elem,
-          "Grabbing frame got interrupted on %s unexpectedly. %d: %s.",
-          v4l2object->videodev, errno, g_strerror (errno));
-      break;
+  /* ERRORS */
+error:
+  {
+    GST_WARNING_OBJECT (pool->v4l2elem,
+        "problem grabbing frame %d (ix=%d), pool-ct=%d, buf.flags=%d",
+        buffer.sequence, buffer.index,
+        GST_MINI_OBJECT_REFCOUNT (pool), buffer.flags);
+
+    switch (errno) {
+      case EAGAIN:
+        GST_WARNING_OBJECT (pool->v4l2elem,
+            "Non-blocking I/O has been selected using O_NONBLOCK and"
+            " no buffer was in the outgoing queue. device %s",
+            v4l2object->videodev);
+        break;
+      case EINVAL:
+        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," " 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,
+            (_("Failed trying to get video frames from device '%s'. Not enough memory."), v4l2object->videodev), (_("insufficient memory to enqueue a user pointer buffer. device %s."), v4l2object->videodev));
+        break;
+      case EIO:
+        GST_INFO_OBJECT (pool->v4l2elem,
+            "VIDIOC_DQBUF failed due to an internal error."
+            " Can also indicate temporary problems like signal loss."
+            " Note the driver might dequeue an (empty) buffer despite"
+            " returning an error, or even stop capturing."
+            " device %s", v4l2object->videodev);
+        /* have we de-queued a buffer ? */
+        if (!(buffer.flags & (V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_DONE))) {
+          GST_DEBUG_OBJECT (pool->v4l2elem, "reenqueing buffer");
+          /* FIXME ... should we do something here? */
+        }
+        break;
+      case EINTR:
+        GST_WARNING_OBJECT (pool->v4l2elem,
+            "could not sync on a buffer on device %s", v4l2object->videodev);
+        break;
+      default:
+        GST_WARNING_OBJECT (pool->v4l2elem,
+            "Grabbing frame got interrupted on %s unexpectedly. %d: %s.",
+            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;
   }
-
-  return NULL;
 }
 
 /**

diff --git a/sys/v4l2/gstv4l2bufferpool.h b/sys/v4l2/gstv4l2bufferpool.h
index 4953806..1cf66d8 100644 (file)
--- a/sys/v4l2/gstv4l2bufferpool.h
+++ b/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;
 

diff --git a/sys/v4l2/gstv4l2sink.c b/sys/v4l2/gstv4l2sink.c
index 9e57762..12e5b7e 100644 (file)
--- a/sys/v4l2/gstv4l2sink.c
+++ b/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);
     }
   }
 

diff --git a/sys/v4l2/gstv4l2src.c b/sys/v4l2/gstv4l2src.c
index a0a5c47..85da17f 100644 (file)
--- a/sys/v4l2/gstv4l2src.c
+++ b/sys/v4l2/gstv4l2src.c
@@ -181,8 +181,8 @@ gst_v4l2src_class_init (GstV4l2SrcClass * klass)
   gst_element_class_set_details_simple (element_class,
       "Video (video4linux2) Source", "Source/Video",
       "Reads frames from a Video4Linux2 device",
-      "Edgard Lima <edgard.lima@indt.org.br>,"
-      " Stefan Kost <ensonic@users.sf.net>");
+      "Edgard Lima <edgard.lima@indt.org.br>, "
+      "Stefan Kost <ensonic@users.sf.net>");
 
   gst_element_class_add_pad_template
       (element_class,
@@ -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
-     * range... */
-    /* We are fixating to greater possble size (limited to GST_V4L2_MAX_SIZE)
+    /* We are fixating to a resonable 320x200 resolution
        and the maximum framerate resolution for that size */
-    gst_structure_fixate_field_nearest_int (structure, "width",
-        GST_V4L2_MAX_SIZE);
-    gst_structure_fixate_field_nearest_int (structure, "height",
-        GST_V4L2_MAX_SIZE);
+    gst_structure_fixate_field_nearest_int (structure, "width", 320);
+    gst_structure_fixate_field_nearest_int (structure, "height", 200);
     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) {
@@ -918,7 +908,7 @@ gst_v4l2src_create (GstPushSrc * src, GstBuffer ** buf)
       v4l2src->ctrl_time += v4l2src->duration;
     } else {
       /* this is not very good (as it should be the next timestamp),
-       * still good enough for linear fades (as long as it is not -1) 
+       * still good enough for linear fades (as long as it is not -1)
        */
       v4l2src->ctrl_time = timestamp;
     }

diff --git a/sys/v4l2/v4l2src_calls.c b/sys/v4l2/v4l2src_calls.c
index c10735c..43c0b48 100644 (file)
--- a/sys/v4l2/v4l2src_calls.c
+++ b/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;