From 851f550003dd18108dbed9482f66753961b3bba5 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Mon, 11 Jul 2011 12:04:57 +0200 Subject: [PATCH] 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 | 221 +++++++++++++++++++------------------------ sys/v4l2/gstv4l2bufferpool.h | 6 ++ sys/v4l2/gstv4l2sink.c | 4 +- sys/v4l2/gstv4l2src.c | 24 ++--- sys/v4l2/v4l2src_calls.c | 2 +- 5 files changed, 114 insertions(+), 143 deletions(-) diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c index adf9cca..1025049 100644 --- a/sys/v4l2/gstv4l2bufferpool.c +++ b/sys/v4l2/gstv4l2bufferpool.c @@ -34,9 +34,7 @@ #include #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 --- 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 --- 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 --- 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 ," - " Stefan Kost "); + "Edgard Lima , " + "Stefan Kost "); 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 --- 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; -- 2.7.4