[Gnash-commit] gnash ChangeLog libmedia/sound_handler.h libmed...

[Bastiaan Jacques]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Bastiaan Jacques <bjacques>     08/02/09 15:06:01

Modified files:
        .              : ChangeLog 
        libmedia       : sound_handler.h 
        libmedia/gst   : SoundHandlerGst.cpp SoundHandlerGst.h 

Log message:
                * libmedia/sound_handler.h: Add a few notes.
                * libmedia/gst/SoundHandlerGst.{h,cpp}: Make SoundHandlerGst 
thread
                safe, because the "loading thread" will call at least 
create_sound.
                Since most methods in the class merely call a SoundGst method,
                introduce a set of member template functions that will a) make 
these
                operations thread safe, b) replace the RV_IF_BAD_HANDLE-style 
macros
                and c) save typing. Fixes bug #22264.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.5603&r2=1.5604
http://cvs.savannah.gnu.org/viewcvs/gnash/libmedia/sound_handler.h?cvsroot=gnash&r1=1.9&r2=1.10
http://cvs.savannah.gnu.org/viewcvs/gnash/libmedia/gst/SoundHandlerGst.cpp?cvsroot=gnash&r1=1.3&r2=1.4
http://cvs.savannah.gnu.org/viewcvs/gnash/libmedia/gst/SoundHandlerGst.h?cvsroot=gnash&r1=1.1&r2=1.2

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.5603
retrieving revision 1.5604
diff -u -b -r1.5603 -r1.5604
--- ChangeLog   9 Feb 2008 08:39:03 -0000       1.5603
+++ ChangeLog   9 Feb 2008 15:05:59 -0000       1.5604
@@ -1,5 +1,15 @@
 2008-02-08 Bastiaan Jacques <address@hidden>
 
+       * libmedia/sound_handler.h: Add a few notes.
+       * libmedia/gst/SoundHandlerGst.{h,cpp}: Make SoundHandlerGst thread
+       safe, because the "loading thread" will call at least create_sound.
+       Since most methods in the class merely call a SoundGst method,
+       introduce a set of member template functions that will a) make these
+       operations thread safe, b) replace the RV_IF_BAD_HANDLE-style macros
+       and c) save typing. Fixes bug #22264.
+
+2008-02-08 Bastiaan Jacques <address@hidden>
+
        * libmedia/gst/SoundGst.cpp: Use mp3parse only if it's available,
        for instance in the absence of plugins-ugly.
 

Index: libmedia/sound_handler.h
===================================================================
RCS file: /sources/gnash/gnash/libmedia/sound_handler.h,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -b -r1.9 -r1.10
--- libmedia/sound_handler.h    8 Feb 2008 15:27:31 -0000       1.9
+++ libmedia/sound_handler.h    9 Feb 2008 15:06:01 -0000       1.10
@@ -15,11 +15,11 @@
 // along with this program; if not, write to the Free Software
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 
-/* $Id: sound_handler.h,v 1.9 2008/02/08 15:27:31 bjacques Exp $ */
+/* $Id: sound_handler.h,v 1.10 2008/02/09 15:06:01 bjacques Exp $ */
 
 /// \page sound_handler_intro Sound handler introduction
 ///
-/// This page must be written, volunteers ? :)
+/// The implementation of this class *must* be thread safe!
 ///
 
 #ifndef SOUND_HANDLER_H
@@ -287,7 +287,7 @@
        ///
        /// @param start
        /// When starting a soundstream from a random frame, this tells where 
in the
-       /// data the decoding should start.
+       /// data the decoding should start, in samples.
        ///
        /// @param envelopes
        /// Some eventsounds have some volume control mechanism called 
envelopes.

Index: libmedia/gst/SoundHandlerGst.cpp
===================================================================
RCS file: /sources/gnash/gnash/libmedia/gst/SoundHandlerGst.cpp,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -b -r1.3 -r1.4
--- libmedia/gst/SoundHandlerGst.cpp    8 Feb 2008 23:21:58 -0000       1.3
+++ libmedia/gst/SoundHandlerGst.cpp    9 Feb 2008 15:06:01 -0000       1.4
@@ -27,27 +27,20 @@
 namespace gnash {
 namespace media {
 
-#define RETURN_IF_BAD_HANDLE(handle)                                          \
-  if (handle < 0 || handle > int(_sounds.size()) - 1) {                       \
-    return;                                                                   \
-  }
 
-#define RV_IF_BAD_HANDLE(handle, value)                                       \
-  if (handle < 0 || handle > int(_sounds.size()) - 1) {                       \
-    return value;                                                             \
-  }
+using boost::bind;
 
 
 SoundHandlerGst::SoundHandlerGst()
   : _timer_id(0)
 {
   gst_init(NULL, NULL);
-  GNASH_REPORT_FUNCTION;
 }
 
-
 SoundHandlerGst::~SoundHandlerGst()
 {
+  boost::mutex::scoped_lock lock(_sounds_mutex); // Just in case...
+
   std::for_each(_sounds.begin(), _sounds.end(), 
boost::checked_deleter<SoundGst>());
   
   _sounds.clear();
@@ -56,14 +49,6 @@
 }
 
 void
-SoundHandlerGst::poll_sounds()
-{
-  std::for_each(_sounds.begin(), _sounds.end(),
-    boost::mem_fn(&SoundGst::poll));
-}
-
-  
-void
 SoundHandlerGst::start_timer()
 {
   if (_timer_id) {
@@ -88,6 +73,8 @@
 SoundHandlerGst::create_sound(void* data, unsigned int data_bytes,
                               std::auto_ptr<SoundInfo> sinfo)
 {
+  boost::mutex::scoped_lock lock(_sounds_mutex);
+
   if (!data) {
     _sounds.push_back(new SoundGst(sinfo));
   } else {
@@ -104,61 +91,45 @@
 SoundHandlerGst::fill_stream_data(unsigned char* data, unsigned int bytes,
                                   unsigned int sample_count, int handle)
 {
-  RV_IF_BAD_HANDLE(handle, 0);
-  
-  return _sounds[handle]->pushData(data, bytes, sample_count);
+  return ts_call(handle,
+                 bind(&SoundGst::pushData, _1, data, bytes, sample_count), 0);
 }
 
 SoundInfo*
 SoundHandlerGst::get_sound_info(int handle)
 {
-  RV_IF_BAD_HANDLE(handle, NULL);
-  
-  return _sounds[handle]->getSoundInfo();
+  return ts_call(handle, bind(&SoundGst::getSoundInfo, _1),
+                 static_cast<SoundInfo*>(NULL));
 }
 
 void
 SoundHandlerGst::play_sound(int handle, int loop_count, int offset,
            long start, const std::vector<sound_envelope>* envelopes)
 {
-  RETURN_IF_BAD_HANDLE(handle);
+  ts_call(handle,
+          bind(&SoundGst::play, _1, loop_count, offset, start, envelopes));
     
   start_timer();
 
-  _sounds[handle]->play(loop_count, offset, start, envelopes);
-  
   _soundsStarted++;
 }
 
-void
-SoundHandlerGst::stop_all_sounds()
-{
-  std::for_each(_sounds.begin(), _sounds.end(), 
boost::mem_fn(&SoundGst::stop));
-}
-
-
 int
 SoundHandlerGst::get_volume(int handle)
 {
-  RV_IF_BAD_HANDLE(handle, 0);
-
-  return _sounds[handle]->getVolume();
+  return ts_call(handle, bind(&SoundGst::getVolume, _1), 0);
 }
        
 void
 SoundHandlerGst::set_volume(int handle, int volume)
 {
-  RETURN_IF_BAD_HANDLE(handle);
-  
-  _sounds[handle]->setVolume(volume);
+  ts_call(handle, bind(&SoundGst::setVolume, _1, volume));  
 }
                
 void
 SoundHandlerGst::stop_sound(int handle)
 {
-  RETURN_IF_BAD_HANDLE(handle);
-  
-  _sounds[handle]->stop();
+  ts_call(handle, bind(&SoundGst::stop, _1));
   
   _soundsStopped++;
 }
@@ -166,7 +137,11 @@
 void
 SoundHandlerGst::delete_sound(int handle)
 {
-  RETURN_IF_BAD_HANDLE(handle);
+  boost::mutex::scoped_lock lock(_sounds_mutex);
+
+  if (handle < 0 || handle > int(_sounds.size()) - 1) {
+    return;
+  }
   
   std::vector<SoundGst*>::iterator it =
     std::find(_sounds.begin(), _sounds.end(), _sounds[handle]);
@@ -181,18 +156,14 @@
 unsigned int
 SoundHandlerGst::get_duration(int handle)
 {
-  RV_IF_BAD_HANDLE(handle, 0);
-  
-  return _sounds[handle]->duration();
 
+  return ts_call(handle, bind(&SoundGst::duration, _1), 0);
 }
 
 unsigned int
 SoundHandlerGst::get_position(int handle)
 {
-  RV_IF_BAD_HANDLE(handle, 0);
-  
-  return _sounds[handle]->position();
+  return ts_call(handle, bind(&SoundGst::position, _1), 0);
 }
 
 
@@ -205,22 +176,20 @@
 void
 SoundHandlerGst::mute()
 {
-  std::for_each(_sounds.begin(), _sounds.end(),
-                boost::mem_fn(&SoundGst::mute));
+  ts_foreach(boost::mem_fn(&SoundGst::mute));
 }
 
 void
 SoundHandlerGst::unmute()
 {
-  std::for_each(_sounds.begin(), _sounds.end(),
-                boost::mem_fn(&SoundGst::unmute));
+  ts_foreach(boost::mem_fn(&SoundGst::unmute));
 }
 
 
 bool
 SoundHandlerGst::is_muted()
 {
-  using namespace boost;
+  boost::mutex::scoped_lock lock(_sounds_mutex);
 
   std::vector<SoundGst*>::iterator it = std::find_if(_sounds.begin(),
     _sounds.end(), bind( std::logical_not<bool>(), 
@@ -229,8 +198,17 @@
   return (it == _sounds.end());
 }
 
-#undef RETURN_IF_BAD_HANDLE
-#undef RV_IF_BAD_HANDLE
+void
+SoundHandlerGst::poll_sounds()
+{
+  ts_foreach(boost::mem_fn(&SoundGst::poll));
+}
+
+void
+SoundHandlerGst::stop_all_sounds()
+{
+  ts_foreach(boost::mem_fn(&SoundGst::stop));
+}
 
 /* static */ as_value
 SoundHandlerGst::poll_cb(const fn_call& /*fn*/)

Index: libmedia/gst/SoundHandlerGst.h
===================================================================
RCS file: /sources/gnash/gnash/libmedia/gst/SoundHandlerGst.h,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -b -r1.1 -r1.2
--- libmedia/gst/SoundHandlerGst.h      8 Feb 2008 15:27:31 -0000       1.1
+++ libmedia/gst/SoundHandlerGst.h      9 Feb 2008 15:06:01 -0000       1.2
@@ -20,6 +20,7 @@
 #include "SoundGst.h"
 #include "timers.h"
 #include "as_value.h"
+#include <boost/thread/mutex.hpp>
 
 namespace gnash {
 namespace media {
@@ -70,8 +71,74 @@
   static as_value poll_cb(const fn_call& fn);
 
 private:
+  
+  /// Thread safe version of for_each which is applied to the _sounds vector.
+  ///
+  /// @param functor Function object of which operator() will be called; akin
+  ///                the third argument of std::for_each.
+  template <typename T>
+  void
+  ts_foreach(T functor)
+  {
+    boost::mutex::scoped_lock lock(_sounds_mutex);
+
+    std::for_each(_sounds.begin(), _sounds.end(), functor);
+  }
+  
+  /// Calls operator() on a function object which takes a SoundGst pointer
+  /// as its sole argument. The pointer is taken from the _sounds object,
+  /// as indicated by the first argument to this template method. The access
+  /// to _sounds is thread safe.
+  /// 
+  /// @param handle the _sounds index to _sounds with which the SoundGst
+  ///               pointer is to be retrieved.
+  /// @param functor the function object to call operator(SoundGst*) on, so 
that
+  ///        the expression functor(SoundGst*) is valid.  
+  template<typename T>
+  void ts_call(int handle, T functor)
+  {
+    boost::mutex::scoped_lock lock(_sounds_mutex);
+
+    if (handle < 0 || handle > int(_sounds.size()) - 1) {
+      return;
+    }
+    
+    functor(_sounds[handle]);
+  }
+  
+  /// This member is like the previous template member, but returns a value
+  /// indicated by its third argument, *if the handle indicated in the first
+  /// argument is invalid*. Otherwise, it returns functor(SoundGst*). Of
+  /// course, the return value of functor() must be equal to the type of 
+  /// bad_handle_rv (the third argument).
+  ///
+  /// @param bad_handle_rv the value to return if the passed handle is invalid.
+  /// @return functor(SoundGst*).  
+  template<typename T, typename R>
+  R ts_call(int handle, T functor, R bad_handle_rv)
+  {
+    boost::mutex::scoped_lock lock(_sounds_mutex);
+
+    if (handle < 0 || handle > int(_sounds.size()) - 1) {
+      return bad_handle_rv;
+    }
+    
+    return functor(_sounds[handle]);
+  }
+
+
+
+
+
+private:
+
+  /// Mutex for access to the _sounds object.
+  boost::mutex _sounds_mutex;
+  
   std::vector<SoundGst*> _sounds;
+  
   unsigned int _timer_id;  
+
 }; // SoundHandlerGst
 
 } // namespace media