Refactor BaseOutput to prepare for better error handling

mopidy/gstreamer.py

@@ -40,7 +40,7 @@ class GStreamer(ThreadingActor):
         self._tee = None
         self._uridecodebin = None
         self._volume = None
-        self._outputs = {}
+        self._outputs = []
 
     def on_start(self):
         self._setup_gstreamer()
@@ -71,7 +71,7 @@ class GStreamer(ThreadingActor):
             self._pipeline.get_by_name('convert').get_pad('sink'))
 
         for output in settings.OUTPUTS:
-            self.connect_output(get_class(output))
+            get_class(output)(self).connect()
 
         # Setup bus and message processor
         bus = self._pipeline.get_bus()
@@ -263,37 +263,34 @@ class GStreamer(ThreadingActor):
         logger.debug('Setting tags to: %s', tags)
         self._taginject.set_property('tags', tags)
 
-    def connect_output(self, cls):
+    def connect_output(self, output):
         """
         Connect output to pipeline.
 
         :param output: output to connect to our pipeline.
-        :type output: :class:`BaseOutput`
+        :type output: :class:`gst.Bin`
         """
-        output = cls().get_bin()
-
         self._pipeline.add(output)
         output.sync_state_with_parent() # Required to add to running pipe
         gst.element_link_many(self._tee, output)
-
+        self._outputs.append(output)
-        self._outputs[output.get_name()] = output
-
         logger.info('Added %s', output.get_name())
 
     def list_outputs(self):
         return self._outputs.keys()
 
-    def remove_output(self, name):
+    def remove_output(self, output):
+        logger.debug('Trying to remove %s', output.get_name())
         if name not in self._outputs:
             return # FIXME raise mopidy exception of some sort?
         src = self._taginject.get_pad('src')
-        src.set_blocked_async(True, self._blocked_callback, name)
+        src.set_blocked_async(True, self._blocked_callback, output)
 
-    def _blocked_callback(self, pad, blocked, name):
+    def _blocked_callback(self, pad, blocked, output):
-        output = self._outputs.pop(name)
         gst.element_unlink_many(self._tee, output)
         output.set_state(gst.STATE_NULL)
         self._pipeline.remove(output)
+        self._outputs.remove(output)
         pad.set_blocked(False)
 
-        logger.warning(u'Removed %s', name)
+        logger.warning(u'Removed %s', output.get_name())

mopidy/outputs/__init__.py

@@ -9,18 +9,29 @@ logger = logging.getLogger('mopidy.outputs')
 class BaseOutput(object):
     """Base class for providing support for multiple pluggable outputs."""
 
-    def get_bin(self):
+    def __init__(self, gstreamer):
+        self.gstreamer = gstreamer
+        self.bin = self.build_bin()
+        self.bin.set_name(self.get_name())
+
+        self.modify_bin()
+
+    def build_bin(self):
         """
         Build output bin that will attached to pipeline.
         """
         description = 'queue ! %s' % self.describe_bin()
         logger.debug('Creating new output: %s', description)
 
-        output = gst.parse_bin_from_description(description, True)
+        return gst.parse_bin_from_description(description, True)
-        output.set_name(self.get_name())
-        self.modify_bin(output)
 
-        return output
+    def connect(self):
+        """Convenience wrapper to attach output to GStreamer pipeline"""
+        self.gstreamer.connect_output(self.bin)
+
+    def remove(self):
+        """Convenience wrapper to remove output from GStreamer pipeline"""
+        self.gstreamer.remove_output(self.bin)
 
     def get_name(self):
         """
@@ -30,16 +41,13 @@ class BaseOutput(object):
         """
         return self.__class__.__name__
 
-    def modify_bin(self, output):
+    def modify_bin(self):
         """
-        Modifies bin before it is installed if needed.
+        Modifies ``self.bin`` before it is installed if needed.
 
         Overriding this method allows for outputs to modify the constructed bin
         before it is installed. This can for instance be a good place to call
         `set_properties` on elements that need to be configured.
-
-        :param output: gst.Bin to modify in some way.
-        :type output: :class:`gst.Bin`
         """
         pass
 

mopidy/outputs/shoutcast.py

@@ -15,8 +15,9 @@ class ShoutcastOutput(BaseOutput):
         return 'audioconvert ! %s ! shout2send name=shoutcast' \
             % settings.SHOUTCAST_OUTPUT_ENCODER
 
-    def modify_bin(self, output):
+    def modify_bin(self):
-        self.set_properties(output.get_by_name('shoutcast'), {
+        shoutcast = self.bin.get_by_name('shoutcast')
+        self.set_properties(shoutcast, {
             u'ip': settings.SHOUTCAST_OUTPUT_SERVER,
             u'mount': settings.SHOUTCAST_OUTPUT_MOUNT,
             u'port': settings.SHOUTCAST_OUTPUT_PORT,