From 655327b05537be5a9f75a32ab45d221c4345a8a6 Mon Sep 17 00:00:00 2001
From: Lyubomir Marinov <lyubomir.marinov@jitsi.org>
Date: Mon, 9 Jul 2012 14:22:15 +0000
Subject: [PATCH] Fixes the switching of the playback device in calls upon
 hotplugging.

---
 .../jitsi/impl/neomedia/MediaServiceImpl.java |  13 ++
 .../impl/neomedia/device/AudioSystem.java     |  30 +++-
 .../media/protocol/portaudio/DataSource.java  |  11 +-
 .../media/renderer/AbstractRenderer.java      |   2 +-
 .../renderer/audio/AbstractAudioRenderer.java | 133 ++++++++++++++++++
 .../renderer/audio/PortAudioRenderer.java     | 127 +++++++++--------
 .../renderer/audio/PulseAudioRenderer.java    |  81 +++++++----
 .../util/event/PropertyChangeNotifier.java    |  41 +++---
 8 files changed, 324 insertions(+), 114 deletions(-)
 create mode 100644 src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/AbstractAudioRenderer.java

diff --git a/src/org/jitsi/impl/neomedia/MediaServiceImpl.java b/src/org/jitsi/impl/neomedia/MediaServiceImpl.java
index 22656317..e7dbe022 100644
--- a/src/org/jitsi/impl/neomedia/MediaServiceImpl.java
+++ b/src/org/jitsi/impl/neomedia/MediaServiceImpl.java
@@ -1386,7 +1386,20 @@ private void deviceConfigurationPropertyChange(PropertyChangeEvent event)
     {
         String propertyName = event.getPropertyName();
 
+        /*
+         * While AUDIO_CAPTURE_DEVICE is sure to affect the DEFAULT_DEVICE,
+         * AUDIO_PLAYBACK_DEVICE is not. Anyway, MediaDevice is supposed to
+         * represent the device to be used for capture AND playback (though its
+         * current implementation MediaDeviceImpl may be incomplete with respect
+         * to the playback representation). Since it is not clear at this point
+         * of the execution whether AUDIO_PLAYBACK_DEVICE really affects the
+         * DEFAULT_DEVICE and for the sake of completeness, throw in the changes
+         * to the AUDIO_NOTIFY_DEVICE as well.
+         */
         if (DeviceConfiguration.AUDIO_CAPTURE_DEVICE.equals(propertyName)
+                || DeviceConfiguration.AUDIO_NOTIFY_DEVICE.equals(propertyName)
+                || DeviceConfiguration.AUDIO_PLAYBACK_DEVICE.equals(
+                        propertyName)
                 || DeviceConfiguration.VIDEO_CAPTURE_DEVICE.equals(
                         propertyName))
         {
diff --git a/src/org/jitsi/impl/neomedia/device/AudioSystem.java b/src/org/jitsi/impl/neomedia/device/AudioSystem.java
index 039ca533..a7d937a1 100644
--- a/src/org/jitsi/impl/neomedia/device/AudioSystem.java
+++ b/src/org/jitsi/impl/neomedia/device/AudioSystem.java
@@ -308,12 +308,22 @@ protected void postInitialize()
             {
                 if (captureDevice != null)
                 {
-                    List<CaptureDeviceInfo> captureDevices = getCaptureDevices();
+                    List<CaptureDeviceInfo> captureDevices
+                        = getCaptureDevices();
 
                     if ((captureDevices == null)
                             || !captureDevices.contains(captureDevice))
                         setCaptureDevice(null, false);
                 }
+                else
+                {
+                    /*
+                     * If captureDevice is null, it means that a device is to be
+                     * used as the default. The default in question may have
+                     * changed.
+                     */
+                    setCaptureDevice(null, false);
+                }
             }
             finally
             {
@@ -330,6 +340,15 @@ protected void postInitialize()
                                     || !notifyDevices.contains(notifyDevice))
                                 setNotifyDevice(null, false);
                         }
+                        else
+                        {
+                            /*
+                             * If notifyDevice is null, it means that a device
+                             * is to be used as the default. The default in
+                             * question may have changed.
+                             */
+                            setNotifyDevice(null, false);
+                        }
                     }
                     finally
                     {
@@ -343,6 +362,15 @@ protected void postInitialize()
                                             playbackDevice))
                                 setPlaybackDevice(null, false);
                         }
+                        else
+                        {
+                            /*
+                             * If playbackDevice is null, it means that a device
+                             * is to be used as the default. The default in
+                             * question may have changed.
+                             */
+                            setPlaybackDevice(null, false);
+                        }
                     }
                 }
             }
diff --git a/src/org/jitsi/impl/neomedia/jmfext/media/protocol/portaudio/DataSource.java b/src/org/jitsi/impl/neomedia/jmfext/media/protocol/portaudio/DataSource.java
index abb21ae9..071af60d 100644
--- a/src/org/jitsi/impl/neomedia/jmfext/media/protocol/portaudio/DataSource.java
+++ b/src/org/jitsi/impl/neomedia/jmfext/media/protocol/portaudio/DataSource.java
@@ -188,14 +188,17 @@ protected void doDisconnect()
      *
      * @return the device index of a PortAudio device identified by the
      * <tt>MediaLocator</tt> of this <tt>DataSource</tt>
+     * @throws IllegalStateException if there is no <tt>MediaLocator</tt>
+     * associated with this <tt>DataSource</tt>
      */
     private int getDeviceIndex()
     {
         MediaLocator locator = getLocator();
 
         if (locator == null)
-            throw new NullPointerException("locator");
-        return getDeviceIndex(locator);
+            throw new IllegalStateException("locator");
+        else
+            return getDeviceIndex(locator);
     }
 
     /**
@@ -211,13 +214,9 @@ public static int getDeviceIndex(MediaLocator locator)
     {
         if (AudioSystem.LOCATOR_PROTOCOL_PORTAUDIO.equalsIgnoreCase(
                 locator.getProtocol()))
-        {
             return Integer.parseInt(locator.getRemainder().replace("#", ""));
-        }
         else
-        {
             throw new IllegalArgumentException("locator.protocol");
-        }
     }
 
     /**
diff --git a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/AbstractRenderer.java b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/AbstractRenderer.java
index 25095725..13689385 100644
--- a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/AbstractRenderer.java
+++ b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/AbstractRenderer.java
@@ -12,7 +12,7 @@
 
 /**
  * Provides an abstract base implementation of <tt>Renderer</tt> in order to
- * facilitate extenders
+ * facilitate extenders.
  *
  * @author Lyubomir Marinov
  *
diff --git a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/AbstractAudioRenderer.java b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/AbstractAudioRenderer.java
new file mode 100644
index 00000000..245932a0
--- /dev/null
+++ b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/AbstractAudioRenderer.java
@@ -0,0 +1,133 @@
+/*
+ * Jitsi, the OpenSource Java VoIP and Instant Messaging client.
+ *
+ * Distributable under LGPL license.
+ * See terms of license at gnu.org.
+ */
+package org.jitsi.impl.neomedia.jmfext.media.renderer.audio;
+
+import java.beans.*;
+
+import javax.media.*;
+import javax.media.format.*;
+
+import org.jitsi.impl.neomedia.device.*;
+import org.jitsi.impl.neomedia.jmfext.media.renderer.*;
+
+/**
+ * Provides an abstract base implementation of <tt>Renderer</tt> which processes
+ * media in <tt>AudioFormat</tt> in order to facilitate extenders.
+ *
+ * @author Lyubomir Marinov
+ */
+public abstract class AbstractAudioRenderer
+    extends AbstractRenderer<AudioFormat>
+{
+    /**
+     * The <tt>AudioSystem</tt> which provides the playback deviced used by this
+     * <tt>Renderer</tt>.
+     */
+    protected final AudioSystem audioSystem;
+
+    /**
+     * The <tt>MediaLocator</tt> which specifies the playback device to be used
+     * by this <tt>Renderer</tt>.
+     */
+    private MediaLocator locator;
+
+    /**
+     * The <tt>PropertyChangeListener</tt> which listens to changes in the
+     * values of the properties of {@link #audioSystem}.
+     */
+    private final PropertyChangeListener propertyChangeListener
+        = new PropertyChangeListener()
+        {
+            public void propertyChange(PropertyChangeEvent event)
+            {
+                AbstractAudioRenderer.this.propertyChange(event);
+            }
+        };
+
+    protected AbstractAudioRenderer(AudioSystem audioSystem)
+    {
+        this.audioSystem = audioSystem;
+    }
+
+    protected AbstractAudioRenderer(String locatorProtocol)
+    {
+        this(AudioSystem.getAudioSystem(locatorProtocol));
+    }
+
+    public void close()
+    {
+        if (audioSystem != null)
+            audioSystem.removePropertyChangeListener(propertyChangeListener);
+    }
+
+    /**
+     * Gets the <tt>MediaLocator</tt> which specifies the playback device to be
+     * used by this <tt>Renderer</tt>.
+     *
+     * @return the <tt>MediaLocator</tt> which specifies the playback device to
+     * be used by this <tt>Renderer</tt>
+     */
+    public MediaLocator getLocator()
+    {
+        MediaLocator locator = this.locator;
+
+        if ((locator == null) && (audioSystem != null))
+        {
+            CaptureDeviceInfo playbackDevice
+                = audioSystem.getPlaybackDevice();
+
+            if (playbackDevice != null)
+                locator = playbackDevice.getLocator();
+        }
+        return locator;
+    }
+
+    public void open()
+        throws ResourceUnavailableException
+    {
+        /*
+         * If this Renderer has not been forced to use a playback device with a
+         * specific MediaLocator, it will use the default playback device (of
+         * its associated AudioSystem). In the case of using the default
+         * playback device, change the playback device used by this instance
+         * upon changes of the default playback device.
+         */
+        if ((this.locator == null) && (audioSystem != null))
+        {
+            MediaLocator locator = getLocator();
+
+            if (locator != null)
+                audioSystem.addPropertyChangeListener(propertyChangeListener);
+        }
+    }
+
+    private void propertyChange(PropertyChangeEvent event)
+    {
+        if (AudioSystem.PROP_PLAYBACK_DEVICE.equals(event.getPropertyName()))
+            reset();
+    }
+
+    /**
+     * Sets the <tt>MediaLocator</tt> which specifies the playback device to be
+     * used by this <tt>Renderer</tt>.
+     *
+     * @param locator the <tt>MediaLocator</tt> which specifies the playback
+     * device to be used by this <tt>Renderer</tt>
+     */
+    public void setLocator(MediaLocator locator)
+    {
+        if (this.locator == null)
+        {
+            if (locator == null)
+                return;
+        }
+        else if (this.locator.equals(locator))
+            return;
+
+        this.locator = locator;
+    }
+}
diff --git a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PortAudioRenderer.java b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PortAudioRenderer.java
index b07930ab..8b1042f1 100644
--- a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PortAudioRenderer.java
+++ b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PortAudioRenderer.java
@@ -6,6 +6,7 @@
  */
 package org.jitsi.impl.neomedia.jmfext.media.renderer.audio;
 
+import java.lang.reflect.*;
 import java.util.*;
 
 import javax.media.*;
@@ -16,7 +17,6 @@
 import org.jitsi.impl.neomedia.*;
 import org.jitsi.impl.neomedia.device.*;
 import org.jitsi.impl.neomedia.jmfext.media.protocol.portaudio.*;
-import org.jitsi.impl.neomedia.jmfext.media.renderer.*;
 import org.jitsi.util.*;
 
 /**
@@ -26,7 +26,7 @@
  * @author Lyubomir Marinov
  */
 public class PortAudioRenderer
-    extends AbstractRenderer<AudioFormat>
+    extends AbstractAudioRenderer
 {
     /**
      * The <tt>Logger</tt> used by the <tt>PortAudioRenderer</tt> class and its
@@ -109,12 +109,6 @@ public class PortAudioRenderer
      */
     private int framesPerBuffer;
 
-    /**
-     * The <tt>MediaLocator</tt> which specifies the device index of the
-     * PortAudio device used by this instance for rendering.
-     */
-    private MediaLocator locator;
-
     private long outputParameters = 0;
 
     /**
@@ -160,6 +154,8 @@ public PortAudioRenderer()
      */
     public PortAudioRenderer(boolean enableVolumeControl)
     {
+        super(AudioSystem.LOCATOR_PROTOCOL_PORTAUDIO);
+
         if (enableVolumeControl)
         {
             /*
@@ -209,6 +205,8 @@ public synchronized void close()
                 PortAudio.PaStreamParameters_free(outputParameters);
                 outputParameters = 0;
             }
+
+            super.close();
         }
     }
 
@@ -225,35 +223,6 @@ public Object[] getControls()
         return new Object[] { gainControl };
     }
 
-    /**
-     * Gets the <tt>MediaLocator</tt> which specifies the device index of the
-     * PortAudio device used by this instance for rendering.
-     *
-     * @return the <tt>MediaLocator</tt> which specifies the device index of the
-     * PortAudio device used by this instance for rendering
-     */
-    public MediaLocator getLocator()
-    {
-        MediaLocator locator = this.locator;
-
-        if (locator == null)
-        {
-            AudioSystem portAudioSystem
-                = AudioSystem.getAudioSystem(
-                        AudioSystem.LOCATOR_PROTOCOL_PORTAUDIO);
-
-            if (portAudioSystem != null)
-            {
-                CaptureDeviceInfo playbackDevice
-                    = portAudioSystem.getPlaybackDevice();
-
-                if (playbackDevice != null)
-                    locator = playbackDevice.getLocator();
-            }
-        }
-        return locator;
-    }
-
     /**
      * Gets the descriptive/human-readable name of this JMF plug-in.
      *
@@ -415,6 +384,8 @@ else if (t instanceof ResourceUnavailableException)
                 throw rue;
             }
         }
+
+        super.open();
     }
 
     /**
@@ -606,6 +577,39 @@ private void process(byte[] buffer, int offset, int length)
         }
     }
 
+    /**
+     * Resets the state of this <tt>PlugIn</tt>.
+     */
+    public synchronized void reset()
+    {
+        waitWhileStreamIsBusy();
+
+        boolean open = (this.stream != 0);
+
+        if (open)
+        {
+            /*
+             * The close method will stop this Renderer if it is currently
+             * started.
+             */
+            boolean start = this.started;
+
+            close();
+
+            try
+            {
+                open();
+            }
+            catch (ResourceUnavailableException rue)
+            {
+                throw new UndeclaredThrowableException(rue);
+            }
+
+            if (start)
+                start();
+        }
+    }
+
     /**
      * Sets the <tt>MediaLocator</tt> which specifies the device index of the
      * PortAudio device to be used by this instance for rendering.
@@ -613,17 +617,11 @@ private void process(byte[] buffer, int offset, int length)
      * @param locator a <tt>MediaLocator</tt> which specifies the device index
      * of the PortAudio device to be used by this instance for rendering
      */
+    @Override
     public void setLocator(MediaLocator locator)
     {
-        if (this.locator == null)
-        {
-            if (locator == null)
-                return;
-        }
-        else if (this.locator.equals(locator))
-            return;
+        super.setLocator(locator);
 
-        this.locator = locator;
         supportedInputFormats = null;
     }
 
@@ -653,35 +651,44 @@ public synchronized void start()
      */
     public synchronized void stop()
     {
-        boolean interrupted = false;
-
-        while (streamIsBusy)
+        waitWhileStreamIsBusy();
+        if (started && (stream != 0))
         {
             try
             {
-                wait();
+                PortAudio.Pa_StopStream(stream);
+                started = false;
+
+                bufferLeft = null;
             }
-            catch (InterruptedException iex)
+            catch (PortAudioException paex)
             {
-                interrupted = true;
+                logger.error("Failed to close PortAudio stream.", paex);
             }
         }
-        if (interrupted)
-            Thread.currentThread().interrupt();
+    }
 
-        if (started && (stream != 0))
+    /**
+     * Waits on this instance while {@link #streamIsBusy} is equal to
+     * <tt>true</tt> i.e. until it becomes <tt>false</tt>. The method should
+     * only be called by a thread that is the owner of this object's monitor.
+     */
+    private void waitWhileStreamIsBusy()
+    {
+        boolean interrupted = false;
+
+        while (streamIsBusy)
         {
             try
             {
-                PortAudio.Pa_StopStream(stream);
-                started = false;
-
-                bufferLeft = null;
+                wait();
             }
-            catch (PortAudioException paex)
+            catch (InterruptedException iex)
             {
-                logger.error("Failed to close PortAudio stream.", paex);
+                interrupted = true;
             }
         }
+        if (interrupted)
+            Thread.currentThread().interrupt();
     }
 }
diff --git a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PulseAudioRenderer.java b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PulseAudioRenderer.java
index 6805a8c3..9f6a47ee 100644
--- a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PulseAudioRenderer.java
+++ b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/PulseAudioRenderer.java
@@ -14,11 +14,15 @@
 
 import org.jitsi.impl.neomedia.*;
 import org.jitsi.impl.neomedia.device.*;
-import org.jitsi.impl.neomedia.jmfext.media.renderer.*;
 import org.jitsi.impl.neomedia.pulseaudio.*;
 
+/**
+ * Implements an audio <tt>Renderer</tt> which uses PulseAudio.
+ *
+ * @author Lyubomir Marinov
+ */
 public class PulseAudioRenderer
-    extends AbstractRenderer<AudioFormat>
+    extends AbstractAudioRenderer
 {
     /**
      * The human-readable <tt>PlugIn</tt> name of the
@@ -53,10 +57,14 @@ public class PulseAudioRenderer
 
     private float gainControlLevel;
 
-    private MediaLocator locator;
-
     private final String mediaRole;
 
+    /*
+     * TODO The field pulseAudioSystem has been introduced prior to
+     * AbstractAudioSystem and its field audioSystem. It would be a good idea to
+     * remove the field pulseAudioSystem in order to reduce the memory footprint
+     * of the PulseAudioRenderer instances.
+     */
     private final PulseAudioSystem pulseAudioSystem;
 
     private long stream;
@@ -80,7 +88,9 @@ public PulseAudioRenderer()
 
     public PulseAudioRenderer(String mediaRole)
     {
-        pulseAudioSystem = PulseAudioSystem.getPulseAudioSystem();
+        super(PulseAudioSystem.getPulseAudioSystem());
+
+        pulseAudioSystem = (PulseAudioSystem) audioSystem;
         if (pulseAudioSystem == null)
             throw new IllegalStateException("pulseAudioSystem");
 
@@ -140,6 +150,8 @@ public void close()
                     PA.stream_unref(stream);
                 }
             }
+
+            super.close();
         }
         finally
         {
@@ -164,21 +176,6 @@ private void cork(boolean b)
         }
     }
 
-    public MediaLocator getLocator()
-    {
-        MediaLocator locator = this.locator;
-
-        if (locator == null)
-        {
-            CaptureDeviceInfo playbackDevice
-                = pulseAudioSystem.getPlaybackDevice();
-
-            if (playbackDevice != null)
-                locator = playbackDevice.getLocator();
-        }
-        return locator;
-    }
-
     private String getLocatorDev()
     {
         MediaLocator locator = getLocator();
@@ -212,6 +209,8 @@ public void open()
         try
         {
             openWithMainloopLock();
+
+            super.open();
         }
         finally
         {
@@ -465,17 +464,43 @@ private int processWithMainloopLock(Buffer buffer)
         return ret;
     }
 
-    public void setLocator(MediaLocator locator)
+    /**
+     * Resets the state of this <tt>PlugIn</tt>.
+     */
+    public void reset()
     {
-        if (this.locator == null)
+        pulseAudioSystem.lockMainloop();
+        try
         {
-            if (locator == null)
-                return;
-        }
-        else if (this.locator.equals(locator))
-            return;
+            boolean open = (this.stream != 0);
+
+            if (open)
+            {
+                /*
+                 * The close method will stop this Renderer if it is currently
+                 * started.
+                 */
+                boolean start = !this.corked;
+
+                close();
 
-        this.locator = locator;
+                try
+                {
+                    open();
+                }
+                catch (ResourceUnavailableException rue)
+                {
+                    throw new UndeclaredThrowableException(rue);
+                }
+
+                if (start)
+                    start();
+            }
+        }
+        finally
+        {
+            pulseAudioSystem.unlockMainloop();
+        }
     }
 
     private void setStreamVolume(long stream, float level)
diff --git a/src/org/jitsi/util/event/PropertyChangeNotifier.java b/src/org/jitsi/util/event/PropertyChangeNotifier.java
index e1555885..77aa1d46 100644
--- a/src/org/jitsi/util/event/PropertyChangeNotifier.java
+++ b/src/org/jitsi/util/event/PropertyChangeNotifier.java
@@ -70,7 +70,9 @@ public void removePropertyChangeListener(PropertyChangeListener listener)
      * <tt>PropertyChangeListener</tt>s registered with this
      * <tt>PropertyChangeNotifier</tt> in order to notify about a change in the
      * value of a specific property which had its old value modified to a
-     * specific new value.
+     * specific new value. <tt>PropertyChangeNotifier</tt> does not check
+     * whether the specified <tt>oldValue</tt> and <tt>newValue</tt> are indeed
+     * different.
      * 
      * @param property the name of the property of this
      * <tt>PropertyChangeNotifier</tt> which had its value changed
@@ -80,28 +82,31 @@ public void removePropertyChangeListener(PropertyChangeListener listener)
      * the change
      */
     protected void firePropertyChange(
-        String property,
-        Object oldValue,
-        Object newValue)
+            String property,
+            Object oldValue,
+            Object newValue)
     {
         PropertyChangeListener[] listeners;
+
         synchronized (this.listeners)
         {
             listeners
-                = this.listeners
-                        .toArray(
-                            new PropertyChangeListener[this.listeners.size()]);
+                = this.listeners.toArray(
+                        new PropertyChangeListener[this.listeners.size()]);
         }
 
-        PropertyChangeEvent event
-            = new PropertyChangeEvent(
-                    getPropertyChangeSource(property, oldValue, newValue),
-                    property,
-                    oldValue,
-                    newValue);
+        if (listeners.length != 0)
+        {
+            PropertyChangeEvent event
+                = new PropertyChangeEvent(
+                        getPropertyChangeSource(property, oldValue, newValue),
+                        property,
+                        oldValue,
+                        newValue);
 
-        for (PropertyChangeListener listener : listeners)
-            listener.propertyChange(event);
+            for (PropertyChangeListener listener : listeners)
+                listener.propertyChange(event);
+        }
     }
 
     /**
@@ -126,9 +131,9 @@ protected void firePropertyChange(
      * specified new value
      */
     protected Object getPropertyChangeSource(
-        String property,
-        Object oldValue,
-        Object newValue)
+            String property,
+            Object oldValue,
+            Object newValue)
     {
         return this;
     }
-- 
GitLab