Fixes a layout and painting issue (seen on Mac OS X) which may lead to toggled local video in calls appearing to be of the wrong bounds.

5 files changed, 193 insertions, 65 deletions

diff --git a/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java b/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java
index 2eced0c..dcbe743 100644
--- a/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java
+++ b/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java
@@ -933,7 +933,7 @@ public class OneToOneCallPeerPanel
      * Handles the change when we turn on/off local video streaming such as
      * creating/releasing visual component.
      *
-     * @param listener Listener that will be callbacked
+     * @param listener Listener that will be called back
      */
     private void handleLocalVideoStreamingChange(
             VideoTelephonyListener listener)
@@ -1782,6 +1782,14 @@ public class OneToOneCallPeerPanel
                         videoContainer.remove(localVideo);
                         videoContainer.remove(closeButton);
                     }
+
+                    /*
+                     * Just like #handleVideoEvent(VideoEvent, Container) says,
+                     * we have to be explicit in order to achieve a proper
+                     * layout and an up-to-date painting.
+                     */
+                    videoContainer.validate();
+                    videoContainer.repaint();
                 }
             }
         }
diff --git a/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java b/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java
index b80e171..0c77a7f 100644
--- a/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java
+++ b/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java
@@ -5,14 +5,12 @@
  */

 package net.java.sip.communicator.impl.neomedia;

 

-import java.util.*;

-

 import java.awt.*;

 import java.awt.event.*;

 import java.io.*;

 

 import javax.media.*;

-import javax.media.MediaException;

+import javax.media.MediaException; // disambiguation

 import javax.swing.*;

 import javax.swing.event.*;

 import javax.swing.table.*;

@@ -25,8 +23,9 @@ import net.java.sip.communicator.util.*;
 import net.java.sip.communicator.util.swing.*;

 

 /**

- * @author Lubomir Marinov

+ * @author Lyubomir Marinov

  * @author Damian Minkov

+ * @author Yana Stamcheva

  */

 public class MediaConfiguration

 {

@@ -134,9 +133,11 @@ public class MediaConfiguration
         final SIPCommCheckBox echoCancelCheckBox = new SIPCommCheckBox(

             NeomediaActivator.getResources().getI18NString(

                 "impl.media.configform.ECHOCANCEL"));

-        // first set the selected one than add the listener

-        // in order to avoid saving tha value when using the default one

-        // and only showing to user without modification

+        /*

+         * First set the selected one, then add the listener in order to avoid

+         * saving the value when using the default one and only showing to user

+         * without modification.

+         */

         echoCancelCheckBox.setSelected(

             mediaService.getDeviceConfiguration().isEchoCancel());

         echoCancelCheckBox.addItemListener(

@@ -155,9 +156,11 @@ public class MediaConfiguration
         final SIPCommCheckBox denoiseCheckBox = new SIPCommCheckBox(

             NeomediaActivator.getResources().getI18NString(

                 "impl.media.configform.DENOISE"));

-        // first set the selected one than add the listener

-        // in order to avoid saving tha value when using the default one

-        // and only showing to user without modification

+        /*

+         * First set the selected one, then add the listener in order to avoid

+         * saving the value when using the default one and only showing to user

+         * without modification.

+         */

         denoiseCheckBox.setSelected(

             mediaService.getDeviceConfiguration().isDenoise());

         denoiseCheckBox.addItemListener(

@@ -207,10 +210,10 @@ public class MediaConfiguration
             {

                 public void itemStateChanged(ItemEvent e)

                 {

-                    if(e.getStateChange() == ItemEvent.SELECTED)

+                    if (ItemEvent.SELECTED == e.getStateChange())

                     {

-                        if(DeviceConfiguration

-                                .AUDIO_SYSTEM_PORTAUDIO.equals(e.getItem()))

+                        if (DeviceConfiguration.AUDIO_SYSTEM_PORTAUDIO.equals(

+                                e.getItem()))

                         {

                             createPortAudioControls(portAudioPanel);

                         }

@@ -224,8 +227,8 @@ public class MediaConfiguration
                     }

                 }

             });

-            if(comboBox.getSelectedItem()

-                .equals(DeviceConfiguration.AUDIO_SYSTEM_PORTAUDIO))

+            if (DeviceConfiguration.AUDIO_SYSTEM_PORTAUDIO.equals(

+                    comboBox.getSelectedItem()))

                 createPortAudioControls(portAudioPanel);

         }

         else

@@ -372,14 +375,16 @@ public class MediaConfiguration
     }

 

     /**

-     * Creates preview for the device(video) in the video container.

+     * Creates preview for the (video) device in the video container.

+     *

      * @param device the device

-     * @param videoContainer the container

-     * @throws IOException a problem accessing the device.

-     * @throws MediaException a problem getting preview.

+     * @param videoContainer the video container

+     * @throws IOException a problem accessing the device

+     * @throws MediaException a problem getting preview

      */

-    private static void createPreview(CaptureDeviceInfo device,

-                               final JComponent videoContainer)

+    private static void createPreview(

+            CaptureDeviceInfo device,

+            JComponent videoContainer)

         throws IOException,

                MediaException

     {

@@ -391,22 +396,23 @@ public class MediaConfiguration
         if (device == null)

             return;

 

-        Iterator<MediaDevice> mDevsIter =

-                NeomediaActivator.getMediaServiceImpl()

-                    .getDevices(MediaType.VIDEO, MediaUseCase.ANY)

-                    .iterator();

-        while(mDevsIter.hasNext())

+        MediaService mediaService = NeomediaActivator.getMediaServiceImpl();

+        Iterable<MediaDevice> devs

+            = mediaService.getDevices(MediaType.VIDEO, MediaUseCase.ANY);

+

+        for (MediaDevice dev : devs)

         {

-            MediaDeviceImpl dev = (MediaDeviceImpl)mDevsIter.next();

-            if(dev.getCaptureDeviceInfo().equals(device))

+            if(((MediaDeviceImpl) dev).getCaptureDeviceInfo().equals(device))

             {

-                Component c = (Component)NeomediaActivator.getMediaServiceImpl()

-                    .getVideoPreviewComponent(

-                            dev,

-                            videoContainer.getSize().width,

-                            videoContainer.getSize().height);

+                Dimension videoContainerSize = videoContainer.getSize();

+                Component preview

+                    = (Component)

+                        mediaService.getVideoPreviewComponent(

+                                dev,

+                                videoContainerSize.width,

+                                videoContainerSize.height);

 

-                videoContainer.add(c);

+                videoContainer.add(preview);

 

                 break;

             }

@@ -492,31 +498,81 @@ public class MediaConfiguration
              * described by pretending there's a selection in the video combo

              * box when the combo box in question becomes displayable.

              */

-            comboBox.addHierarchyListener(new HierarchyListener()

+            HierarchyListener hierarchyListener = new HierarchyListener()

             {

+                private Window window;

+

+                private WindowListener windowListener;

+

+                public void dispose()

+                {

+                    if (windowListener != null)

+                    {

+                        if (window != null)

+                        {

+                            window.removeWindowListener(windowListener);

+                            window = null;

+                        }

+                        windowListener = null;

+                    }

+

+                    videoDeviceInPreview = null;

+                }

+

                 public void hierarchyChanged(HierarchyEvent event)

                 {

-                    if (((event.getChangeFlags()

-                                    & HierarchyEvent.DISPLAYABILITY_CHANGED)

-                                != 0)

-                            && comboBox.isDisplayable())

+                    if ((event.getChangeFlags()

+                                & HierarchyEvent.DISPLAYABILITY_CHANGED)

+                            == 0)

+                        return;

+

+                    if (comboBox.isDisplayable())

                     {

-                        // let current changes end their execution

-                        // and after that trigger action on combobox

-                        SwingUtilities.invokeLater(new Runnable(){

+                        /*

+                         * Let current changes end their execution and trigger

+                         * action on combobox afterwards.

+                         */

+                        SwingUtilities.invokeLater(new Runnable()

+                        {

                             public void run()

                             {

                                 comboBoxListener.actionPerformed(null);

                             }

                         });

+

+                        /*

+                         * FIXME When the Options dialog closes on Mac OS X, the

+                         * displayable property of the comboBox will not become

+                         * false. Consequently, the next time the Options dialog

+                         * opens, the displayable property will not change.

+                         * Which will lead to no preview being created for the

+                         * device selected in the comboBox.

+                         */

+                        if (windowListener == null)

+                        {

+                            window

+                                = SwingUtilities.windowForComponent(comboBox);

+                            if (window != null)

+                            {

+                                windowListener = new WindowAdapter()

+                                {

+                                    @Override

+                                    public void windowClosing(WindowEvent event)

+                                    {

+                                        dispose();

+                                    }

+                                };

+                                window.addWindowListener(windowListener);

+                            }

+                        }

                     }

                     else

                     {

-                        if(!comboBox.isDisplayable())

-                            videoDeviceInPreview = null;

+                        dispose();

                     }

                 }

-            });

+            };

+            comboBox.addHierarchyListener(hierarchyListener);

         } else

             preview = new TransparentPanel();

 

diff --git a/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java b/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java
index 71bf547..f150646 100644
--- a/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java
+++ b/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java
@@ -692,8 +692,9 @@ public class MediaServiceImpl
      * @param preferredHeight the height we prefer for the component
      * @return the preview component.
      */
-    public Object getVideoPreviewComponent(MediaDevice device
-                    , int preferredWidth, int preferredHeight)
+    public Object getVideoPreviewComponent(
+            MediaDevice device,
+            int preferredWidth, int preferredHeight)
     {
         JLabel noPreview =
         new JLabel(NeomediaActivator.getResources().getI18NString(
@@ -720,16 +721,17 @@ public class MediaServiceImpl
                 ((MediaDeviceImpl)device).getCaptureDeviceInfo().getLocator());
 
             /*
-            * Don't let the size be uselessly small just because the videoContainer
-            * has too small a preferred size.
-            */
+             * Don't let the size be uselessly small just because the
+             * videoContainer has too small a preferred size.
+             */
             if ((preferredWidth < 128) || (preferredHeight < 96))
             {
                 preferredHeight = 128;
                 preferredWidth = 96;
             }
-            VideoMediaStreamImpl
-                .selectVideoSize(dataSource, preferredWidth, preferredHeight);
+            VideoMediaStreamImpl.selectVideoSize(
+                    dataSource,
+                    preferredWidth, preferredHeight);
 
             // A Player is documented to be created on a connected DataSource.
             dataSource.connect();
diff --git a/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java b/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java
index c21b0cb..7647b1e 100644
--- a/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java
+++ b/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java
@@ -683,7 +683,15 @@ public class JAWTRenderer
                 @Override
                 public void componentResized(ComponentEvent e)
                 {
-                    processLightweightComponentEvent();
+                    /*
+                     * It is necessary to call
+                     * #procesLightweightComponentEvent() when the parent gets
+                     * resized only if the native counterpart of this
+                     * SwingVideoComponent expects bounds in a coordinate system
+                     * which changes with respect to the bounds of this
+                     * SwingVideoComponent when the parent gets resized. 
+                     */
+                    //processLightweightComponentEvent();
                 }
             };
 
@@ -927,7 +935,7 @@ public class JAWTRenderer
     {
         /**
          * The handle to the native <tt>JAWTRenderer</tt> which does the actual
-         * paiting of this <tt>SwingVideoComponentCanvas</tt>.
+         * painting of this <tt>SwingVideoComponentCanvas</tt>.
          */
         private long handle;
 
diff --git a/src/net/java/sip/communicator/util/swing/VideoContainer.java b/src/net/java/sip/communicator/util/swing/VideoContainer.java
index a387a7a..2042337 100644
--- a/src/net/java/sip/communicator/util/swing/VideoContainer.java
+++ b/src/net/java/sip/communicator/util/swing/VideoContainer.java
@@ -9,8 +9,6 @@ package net.java.sip.communicator.util.swing;
 import java.awt.*;

 import java.lang.reflect.*;

 

-import javax.swing.*;

-

 import net.java.sip.communicator.util.*;

 

 /**

@@ -32,6 +30,15 @@ public class VideoContainer
     private static final Logger logger = Logger.getLogger(VideoContainer.class);

 

     /**

+     * The name of the instance method of <tt>Component</tt>s added to

+     * <tt>VideoContainer</tt> which creates a new <tt>Component</tt> acting as

+     * a canvas in which the other <tt>Component</tt>s contained in the

+     * <tt>VideoContainer</tt> are painted.

+     */

+    private static final String VIDEO_CANVAS_FACTORY_METHOD_NAME

+        = "createCanvas";

+

+    /**

      * The <tt>Component</tt> which is the canvas, if any, in which the other

      * <tt>Component</tt>s contained in this <tt>VideoContainer</tt> are

      * painted.

@@ -102,6 +109,14 @@ public class VideoContainer
     @Override

     public void add(Component comp, Object constraints, int index)

     {

+        if (VideoLayout.CENTER_REMOTE.equals(constraints)

+                && (noVideoComponent != null)

+                && !noVideoComponent.equals(comp))

+        {

+            remove(noVideoComponent);

+            validate();

+        }

+

         if ((canvas == null) || (canvas.getParent() != this))

         {

             if (OSUtils.IS_MAC && (comp != canvas))

@@ -119,7 +134,9 @@ public class VideoContainer
 

                 try

                 {

-                    Method m = comp.getClass().getMethod("createCanvas");

+                    Method m

+                        = comp.getClass().getMethod(

+                                VIDEO_CANVAS_FACTORY_METHOD_NAME);

 

                     if (m != null)

                     {

@@ -179,13 +196,11 @@ public class VideoContainer
                 index++;

         }

 

-        if (VideoLayout.CENTER_REMOTE.equals(constraints)

-                && (noVideoComponent != null)

-                && !noVideoComponent.equals(comp))

-        {

-            remove(noVideoComponent);

-            validate();

-        }

+        /*

+         * XXX Do not call #remove(Component) beyond this point and before

+         * #add(Component, Object, int) because #removeCanvasIfNecessary() will

+         * remove the canvas. 

+         */

 

         super.add(comp, constraints, index);

     }

@@ -204,7 +219,10 @@ public class VideoContainer
         if ((comp == canvas)

                 && (canvas != null)

                 && (canvas.getParent() != this))

+        {

             canvas = null;

+            validate();

+        }

 

         if (VideoLayout.CENTER_REMOTE.equals(

                         ((VideoLayout) getLayout()).getComponentConstraints(

@@ -215,6 +233,8 @@ public class VideoContainer
             add(noVideoComponent, VideoLayout.CENTER_REMOTE);

             validate();

         }

+

+        removeCanvasIfNecessary();

     }

 

     /**

@@ -237,4 +257,38 @@ public class VideoContainer
         add(noVideoComponent, VideoLayout.CENTER_REMOTE);

         validate();

     }

+

+    /**

+     * Removes {@link #canvas} from this <tt>VideoContainer</tt> if no sibling

+     * <tt>Component</tt> needs it.

+     */

+    public void removeCanvasIfNecessary()

+    {

+        if ((canvas == null) || !OSUtils.IS_MAC)

+            return;

+

+        boolean removeCanvas = true;

+

+        for (Component component : getComponents())

+        {

+            if (component == canvas)

+                continue;

+            try

+            {

+                component.getClass().getMethod(

+                        VIDEO_CANVAS_FACTORY_METHOD_NAME);

+                removeCanvas = false;

+                break;

+            }

+            catch (NoSuchMethodException nsme)

+            {

+                /*

+                 * Ignore it because we already presume that component does not

+                 * need the canvas.

+                 */

+            }

+        }

+        if (removeCanvas)

+            remove(canvas);

+    }

 }