[gstreamermm] Clean up RingBuffer code and avoid warning



commit 06dd512141aece65e971138b663091d280ed6fe6
Author: Daniel Elstner <danielk openismus com>
Date:   Tue Oct 6 13:05:24 2009 +0200

    Clean up RingBuffer code and avoid warning
    
    * gstreamer/src/ringbuffer.hg: Annotate API with a few TODO comments.
    * gstreamer/src/ringbuffer.ccg: Include STL <cstring> header instead
    of <string.h>.  Place file-scope declarations in anonymous namespace.
    (RingBuffer_Fill_gstreamermm_callback): Remove unused parameter name
    to get rid of a compiler warning.  Use extern "C" calling convention.
    (RingBuffer::copy_fields_from): Replace C-style casts with C++ casts.
    (copy_fields_to): Likewise.  Unconditionally assign spec.caps field.
    (set_fill_slot): Comment on a couple of problems with the code.
    (parse_caps): Use Glib::unwrap() to handle a null RefPtr<>.

 ChangeLog                    |   14 +++++++++++
 gstreamer/src/ringbuffer.ccg |   53 +++++++++++++++++++++++++----------------
 gstreamer/src/ringbuffer.hg  |   15 ++++++++----
 3 files changed, 56 insertions(+), 26 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 47e96f8..7fe4b31 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2009-10-06  Daniel Elstner  <danielk openismus com>
+
+	Clean up RingBuffer code and avoid warning
+
+	* gstreamer/src/ringbuffer.hg: Annotate API with a few TODO comments.
+	* gstreamer/src/ringbuffer.ccg: Include STL <cstring> header instead
+	of <string.h>.  Place file-scope declarations in anonymous namespace.
+	(RingBuffer_Fill_gstreamermm_callback): Remove unused parameter name
+	to get rid of a compiler warning.  Use extern "C" calling convention.
+	(RingBuffer::copy_fields_from): Replace C-style casts with C++ casts.
+	(copy_fields_to): Likewise.  Unconditionally assign spec.caps field.
+	(set_fill_slot): Comment on a couple of problems with the code.
+	(parse_caps): Use Glib::unwrap() to handle a null RefPtr<>.
+
 2009-10-06  Daniel Elstner  <daniel kitta gmail com>
 
 	Further streamline the C++ plugin generator code
diff --git a/gstreamer/src/ringbuffer.ccg b/gstreamer/src/ringbuffer.ccg
index 718f545..c8f8907 100644
--- a/gstreamer/src/ringbuffer.ccg
+++ b/gstreamer/src/ringbuffer.ccg
@@ -17,28 +17,37 @@
  * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
-#include <string.h>
+#include <cstring>
 #include <gst/audio/audio-enumtypes.h>
 _PINCLUDE(gstreamermm/private/object_p.h)
 
-static void RingBuffer_Fill_gstreamermm_callback(GstRingBuffer* rbuf, guint8* data, guint len, gpointer user_data)
+namespace
 {
-  Gst::RingBuffer::SlotFill* the_slot = static_cast<Gst::RingBuffer::SlotFill*>(user_data);
+extern "C"
+{
+
+static void RingBuffer_Fill_gstreamermm_callback(GstRingBuffer*, guint8* data, guint len,
+                                                 gpointer user_data)
+{
+  Gst::RingBuffer::SlotFill& slot_fill = *static_cast<Gst::RingBuffer::SlotFill*>(user_data);
 
-  #ifdef GLIBMM_EXCEPTIONS_ENABLED
+#ifdef GLIBMM_EXCEPTIONS_ENABLED
   try
+#endif
   {
-  #endif //GLIBMM_EXCEPTIONS_ENABLED
-    (*the_slot)(data, len);
-  #ifdef GLIBMM_EXCEPTIONS_ENABLED
+    slot_fill(data, len);
   }
-  catch(...)
+#ifdef GLIBMM_EXCEPTIONS_ENABLED
+  catch (...)
   {
     Glib::exception_handlers_invoke();
   }
-  #endif //GLIBMM_EXCEPTIONS_ENABLED
+#endif
 }
 
+} // extern "C"
+} // anonymous namespace
+
 namespace Gst
 {
 
@@ -57,8 +66,8 @@ void RingBufferSpec::copy_fields_from(const GstRingBufferSpec& spec)
 {
   caps = Glib::wrap(spec.caps);
 
-  type = (Gst::BufferFormatType)(spec.type);
-  format = (Gst::BufferFormat)(spec.format);
+  type = static_cast<Gst::BufferFormatType>(spec.type);
+  format = static_cast<Gst::BufferFormat>(spec.format);
   sign = spec.sign;
   bigend = spec.bigend;
   width = spec.width;
@@ -72,16 +81,14 @@ void RingBufferSpec::copy_fields_from(const GstRingBufferSpec& spec)
   bytes_per_sample = spec.bytes_per_sample;
   seglatency = spec.seglatency;
 
-  memcpy(silence_sample, spec.silence_sample, sizeof(silence_sample));
+  std::memcpy(silence_sample, spec.silence_sample, sizeof(silence_sample));
 }
 
 void RingBufferSpec::copy_fields_to(GstRingBufferSpec& spec) const
 {
-  if(caps)
-    spec.caps = caps->gobj();
-
-  spec.type = (GstBufferFormatType)(type);
-  spec.format = (GstBufferFormat)(format);
+  spec.caps = Glib::unwrap(caps);
+  spec.type = static_cast<GstBufferFormatType>(type);
+  spec.format = static_cast<GstBufferFormat>(format);
   spec.sign = sign;
   spec.bigend = bigend;
   spec.width = width;
@@ -95,7 +102,7 @@ void RingBufferSpec::copy_fields_to(GstRingBufferSpec& spec) const
   spec.bytes_per_sample = bytes_per_sample;
   spec.seglatency = seglatency;
 
-  memcpy(spec.silence_sample, silence_sample, sizeof(spec.silence_sample));
+  std::memcpy(spec.silence_sample, silence_sample, sizeof(spec.silence_sample));
 }
 
 RingBuffer::RingBuffer(const Glib::ConstructParams& construct_params)
@@ -106,12 +113,16 @@ RingBuffer::RingBuffer(const Glib::ConstructParams& construct_params)
 
 RingBuffer::RingBuffer(GstRingBuffer* castitem)
 :
-  Gst::Object((GstObject*)(castitem)),
+  Gst::Object(reinterpret_cast<GstObject*>(castitem)),
   _slot_set(false)
 {}
 
 void RingBuffer::set_fill_slot(const SlotFill& slot)
 {
+  // TODO: Not safe against self assignment, and not exception-safe.  Is the
+  // separate _slot_set flag really needed?  Is dynamic allocation actually
+  // needed at all?  This would not have to be a member at all if there were
+  // an accessor to the callback user data in the C API.
   if(_slot_set)
     delete this->slot;
 
@@ -133,9 +144,9 @@ bool RingBuffer::parse_caps(Gst::RingBufferSpec& spec, const Glib::RefPtr<Gst::C
 {
   GstRingBufferSpec gst_spec;
   spec.copy_fields_to(gst_spec);
-  gboolean const result = gst_ring_buffer_parse_caps(&gst_spec, caps->gobj());
+  gboolean const result = gst_ring_buffer_parse_caps(&gst_spec, Glib::unwrap(caps));
   spec.copy_fields_from(gst_spec);
-  return result;
+  return (result != 0);
 }
 
 RingBuffer::~RingBuffer()
diff --git a/gstreamer/src/ringbuffer.hg b/gstreamer/src/ringbuffer.hg
index fe2364c..c29d7ae 100644
--- a/gstreamer/src/ringbuffer.hg
+++ b/gstreamer/src/ringbuffer.hg
@@ -32,8 +32,11 @@ _WRAP_ENUM(RingBufferState, GstRingBufferState)
 _WRAP_ENUM(BufferFormat, GstBufferFormat)
 _WRAP_ENUM(BufferFormatType, GstBufferFormatType)
 
+// TODO: Rewrite at next API break, either as a real class with accessors
+// instead of public data members, or alternatively as a plain struct
+// accompanied by a justification.
 /** The structure containing the format specification of a Gst::RingBuffer.
- * see Gst::RingBuffer::acquire().
+ * @see Gst::RingBuffer::acquire().
  */
 class RingBufferSpec
 {
@@ -94,11 +97,12 @@ public:
   int           segtotal;
 
   /** Number of bytes of one sample.
-  */
+   */
   int           bytes_per_sample;
 
+  // TODO: Ouch.  No way.
   /** Bytes representing one sample of silence.
-  */
+   */
   guint8        silence_sample[32];
 
   /** Number of segments queued in the lower level device, defaults to
@@ -124,7 +128,7 @@ public:
  * Last reviewed on 2006-02-02 (0.10.4).
  * @ingroup GstBaseClasses
  */
-class RingBuffer : public Gst::Object 
+class RingBuffer : public Gst::Object
 {
   _CLASS_GOBJECT(RingBuffer, GstRingBuffer, GST_RING_BUFFER, Gst::Object, GstObject)
   _CUSTOM_DTOR()
@@ -157,7 +161,7 @@ public:
    *
    * @param spec The specs of the buffer.
    *
-   * @return true if the device could be acquired, false on error. MT safe. 
+   * @return true if the device could be acquired, false on error. MT safe.
    */
   bool acquire(const Gst::RingBufferSpec& spec);
   _IGNORE(gst_ring_buffer_acquire)
@@ -199,6 +203,7 @@ public:
 
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
 private:
+  // TODO: Follow naming conventions and use std::auto_ptr<SlotFill>.
   SlotFill* slot;
   bool _slot_set;
 #endif



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]