From e0e1a27e951b0278e438910f157a1695d4dab9c6 Mon Sep 17 00:00:00 2001
From: Lyubomir Marinov <lyubomir.marinov@jitsi.org>
Date: Wed, 24 Oct 2012 07:07:18 +0000
Subject: [PATCH] Fixes exceptions, a memory leak due to incomplete code.
---
.../media/renderer/video/JAWTRenderer.java | 13 +-
.../impl/neomedia/RTPTranslatorImpl.java | 18 +-
.../device/VideoMediaDeviceSession.java | 50 +++-
src/org/jitsi/util/swing/FitLayout.java | 9 +-
src/org/jitsi/util/swing/VideoLayout.java | 253 +++++++++++++-----
5 files changed, 256 insertions(+), 87 deletions(-)
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 07b66f37..f1568666 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
@@ -18,6 +18,7 @@
import org.jitsi.service.configuration.*;
import org.jitsi.service.libjitsi.*;
import org.jitsi.util.*;
+import org.jitsi.util.swing.*;
/**
* Implements a <tt>VideoRenderer</tt> which uses JAWT to perform native
@@ -595,13 +596,19 @@ public Format setInputFormat(Format format)
* Apart from the simplest of cases in which the component has no
* preferredSize, it is also necessary to reflect the width and
* height of the input onto the preferredSize when the ratio of the
- * input is different than the ratio of the preferredSize.
+ * input is different than the ratio of the preferredSize. It may
+ * also be argued that the component needs to know of the width and
+ * height of the input if its preferredSize is with the same ratio
+ * but is smaller.
*/
if ((preferredSize == null)
|| (preferredSize.width < 1)
|| (preferredSize.height < 1)
- || (preferredSize.width * inputHeight
- != preferredSize.height * inputWidth))
+ || !VideoLayout.areAspectRatiosEqual(
+ preferredSize,
+ inputWidth, inputHeight)
+ || (preferredSize.width < inputWidth)
+ || (preferredSize.height < inputHeight))
{
component.setPreferredSize(
new Dimension(inputWidth, inputHeight));
diff --git a/src/org/jitsi/impl/neomedia/RTPTranslatorImpl.java b/src/org/jitsi/impl/neomedia/RTPTranslatorImpl.java
index 2a6134aa..4a039be0 100644
--- a/src/org/jitsi/impl/neomedia/RTPTranslatorImpl.java
+++ b/src/org/jitsi/impl/neomedia/RTPTranslatorImpl.java
@@ -319,7 +319,23 @@ public synchronized SendStream createSendStream(
public synchronized void dispose()
{
manager.removeReceiveStreamListener(this);
- manager.dispose();
+ try
+ {
+ manager.dispose();
+ }
+ catch (Throwable t)
+ {
+ if (t instanceof ThreadDeath)
+ throw (ThreadDeath) t;
+ else
+ {
+ /*
+ * RTPManager.dispose() often throws at least a
+ * NullPointerException in relation to some RTP BYE.
+ */
+ logger.error("Failed to dispose of RTPManager", t);
+ }
+ }
}
public synchronized void dispose(StreamRTPManager streamRTPManager)
diff --git a/src/org/jitsi/impl/neomedia/device/VideoMediaDeviceSession.java b/src/org/jitsi/impl/neomedia/device/VideoMediaDeviceSession.java
index 2470f221..99271d51 100644
--- a/src/org/jitsi/impl/neomedia/device/VideoMediaDeviceSession.java
+++ b/src/org/jitsi/impl/neomedia/device/VideoMediaDeviceSession.java
@@ -32,6 +32,7 @@
import org.jitsi.service.resources.*;
import org.jitsi.util.*;
import org.jitsi.util.event.*;
+import org.jitsi.util.swing.*;
/**
* Extends <tt>MediaDeviceSession</tt> to add video-specific functionality.
@@ -1132,13 +1133,42 @@ public void run()
if (visualComponent != null)
{
- fireVideoEvent(
- new SizeChangeVideoEvent(
- this,
- visualComponent,
- origin,
- width, height),
- false);
+ /*
+ * The Player will notice the new size and will notify about it
+ * before it reaches the Renderer. The notification/event may as
+ * well arrive before the Renderer reflects the new size onto the
+ * preferredSize of the Component. In order to make sure that the
+ * new size is reflected on the preferredSize of the Component
+ * before the notification/event arrives to its
+ * destination/listener, reflect it as soon as possible i.e. now.
+ */
+ try
+ {
+ Dimension preferredSize = visualComponent.getPreferredSize();
+
+ if ((preferredSize == null)
+ || (preferredSize.width < 1)
+ || (preferredSize.height < 1)
+ || !VideoLayout.areAspectRatiosEqual(
+ preferredSize,
+ width, height)
+ || (preferredSize.width < width)
+ || (preferredSize.height < height))
+ {
+ visualComponent.setPreferredSize(
+ new Dimension(width, height));
+ }
+ }
+ finally
+ {
+ fireVideoEvent(
+ new SizeChangeVideoEvent(
+ this,
+ visualComponent,
+ origin,
+ width, height),
+ false);
+ }
}
}
@@ -1245,10 +1275,8 @@ private void playerVisualComponentResized(
int outputHeightDelta
= outputSize.height - playerScalerOutputSize.height;
- if ((outputWidthDelta < -1)
- || (outputWidthDelta > 1)
- || (outputHeightDelta < -1)
- || (outputHeightDelta > 1))
+ if ((outputWidthDelta < -1) || (outputWidthDelta > 1)
+ || (outputHeightDelta < -1) || (outputHeightDelta > 1))
{
playerScaler.setOutputSize(outputSize);
}
diff --git a/src/org/jitsi/util/swing/FitLayout.java b/src/org/jitsi/util/swing/FitLayout.java
index fb7615d8..6af174e3 100644
--- a/src/org/jitsi/util/swing/FitLayout.java
+++ b/src/org/jitsi/util/swing/FitLayout.java
@@ -25,9 +25,12 @@ public class FitLayout
implements LayoutManager
{
- /*
- * Does nothing because this LayoutManager lays out only the first Component
- * of the parent Container and thus doesn't need String associations.
+ /**
+ * {@inheritDoc}
+ *
+ * Does nothing because this <tt>LayoutManager</tt> lays out only the first
+ * <tt>Component</tt> of the parent <tt>Container</tt> and thus doesn't need
+ * any <tt>String</tt> associations.
*/
public void addLayoutComponent(String name, Component comp)
{
diff --git a/src/org/jitsi/util/swing/VideoLayout.java b/src/org/jitsi/util/swing/VideoLayout.java
index 37c96bcc..d3a94827 100644
--- a/src/org/jitsi/util/swing/VideoLayout.java
+++ b/src/org/jitsi/util/swing/VideoLayout.java
@@ -77,7 +77,7 @@ public class VideoLayout
/**
* The map of component constraints.
*/
- private final HashMap<Component, Object> constraints
+ private final Map<Component, Object> constraints
= new HashMap<Component, Object>();
/**
@@ -122,17 +122,19 @@ public void addLayoutComponent(String name, Component comp)
synchronized (constraints)
{
- this.constraints.put(comp, name);
+ constraints.put(comp, name);
}
if ((name == null) || name.equals(CENTER_REMOTE))
{
- remotes.add(comp);
+ if (!remotes.contains(comp))
+ remotes.add(comp);
remoteAlignmentX = Component.CENTER_ALIGNMENT;
}
else if (name.equals(EAST_REMOTE))
{
- remotes.add(comp);
+ if (!remotes.contains(comp))
+ remotes.add(comp);
remoteAlignmentX = Component.RIGHT_ALIGNMENT;
}
else if (name.equals(LOCAL))
@@ -143,6 +145,39 @@ else if (name.equals(CANVAS))
canvas = comp;
}
+ /**
+ * Determines whether the aspect ratio of a specific <tt>Dimension</tt> is
+ * to be considered equal to the aspect ratio of specific <tt>width</tt> and
+ * <tt>height</tt>.
+ *
+ * @param size the <tt>Dimension</tt> whose aspect ratio is to be compared
+ * to the aspect ratio of <tt>width</tt> and <tt>height</tt>
+ * @param width the width which defines in combination with <tt>height</tt>
+ * the aspect ratio to be compared to the aspect ratio of <tt>size</tt>
+ * @param height the height which defines in combination with <tt>width</tt>
+ * the aspect ratio to be compared to the aspect ratio of <tt>size</tt>
+ * @return <tt>true</tt> if the aspect ratio of <tt>size</tt> is to be
+ * considered equal to the aspect ratio of <tt>width</tt> and
+ * <tt>height</tt>; otherwise, <tt>false</tt>
+ */
+ public static boolean areAspectRatiosEqual(
+ Dimension size,
+ int width, int height)
+ {
+ if (size.height == 0)
+ return (height == 0);
+ else if (height == 0)
+ return false;
+ else
+ {
+ double a = size.width / size.height;
+ double b = width / height;
+ double difference = a - b;
+
+ return (-0.01 < difference) && (difference < 0.01);
+ }
+ }
+
/**
* Determines how may columns to use for the grid display of specific remote
* visual/video <tt>Component</tt>s.
@@ -210,26 +245,35 @@ public Component getLocalCloseButton()
}
/**
- * Lays out this given container.
+ * Lays out the specified <tt>Container</tt> (i.e. the <tt>Component</tt>s
+ * it contains) in accord with the logic implemented by this
+ * <tt>LayoutManager</tt>.
*
- * @param parent the container to lay out
+ * @param parent the <tt>Container</tt> to lay out
*/
@Override
public void layoutContainer(Container parent)
{
+ /*
+ * XXX The methods layoutContainer and preferredLayoutSize must be kept
+ * in sync.
+ */
+
List<Component> remotes;
Component local = getLocal();
/*
* When there are multiple remote visual/video Components, the local one
* will be displayed as if it is a remote one i.e. in the same grid, not
- * on top of a remote one.
+ * on top of a remote one. The same layout will be used when this
+ * instance is dedicated to a telephony conference.
*/
- if ((this.remotes.size() > 1) && (local != null))
+ if (conference || ((this.remotes.size() > 1) && (local != null)))
{
remotes = new ArrayList<Component>();
remotes.addAll(this.remotes);
- remotes.add(local);
+ if (local != null)
+ remotes.add(local);
}
else
remotes = this.remotes;
@@ -237,9 +281,17 @@ public void layoutContainer(Container parent)
int remoteCount = remotes.size();
Dimension parentSize = parent.getSize();
- if ((remoteCount == 1) && !conference)
+ if (!conference && (remoteCount == 1))
{
- super.layoutContainer(parent,
+ /*
+ * If the videos are to be laid out as in a one-to-one call, the
+ * remote video has to fill the parent and the local video will be
+ * placed on top of the remote video. The remote video will be laid
+ * out now and the local video will be laid out later/further
+ * bellow.
+ */
+ super.layoutContainer(
+ parent,
(local == null)
? Component.CENTER_ALIGNMENT
: remoteAlignmentX);
@@ -305,7 +357,7 @@ else if (remoteCount > 0)
* If the local visual/video Component is not displayed as if it is
* a remote one, it will be placed on top of a remote one.
*/
- if (!remotes.contains(local) && !conference)
+ if (!remotes.contains(local))
{
Component remote0 = remotes.isEmpty() ? null : remotes.get(0);
int localX;
@@ -321,7 +373,7 @@ else if (remoteCount > 0)
* that there is no remote video and the remote is the
* photoLabel.
*/
- if ((remotes.size() == 1) && (remote0 instanceof JLabel))
+ if ((remoteCount == 1) && (remote0 instanceof JLabel))
{
localX = (parentSize.width - width) / 2;
localY = parentSize.height - height;
@@ -340,6 +392,7 @@ else if (remoteCount > 0)
Component.BOTTOM_ALIGNMENT);
}
+ /* The closeButton has to be on top of the local video. */
if (closeButton != null)
{
/*
@@ -363,26 +416,12 @@ else if (remoteCount > 0)
}
}
+ /*
+ * The video canvas will get the locations of the other components to
+ * paint so it has to cover the parent completely.
+ */
if (canvas != null)
- {
- /*
- * The video canvas will get the locations of the other components
- * to paint so it has to cover the parent completely.
- */
canvas.setBounds(0, 0, parentSize.width, parentSize.height);
- }
- }
-
- /**
- * {@inheritDoc}
- *
- * The <tt>VideoLayout</tt> implementation of the method does nothing.
- */
- @Override
- public Dimension minimumLayoutSize(Container parent)
- {
- // TODO Auto-generated method stub
- return super.minimumLayoutSize(parent);
}
/**
@@ -401,73 +440,144 @@ public Dimension preferredLayoutSize(Container parent)
/*
* When there are multiple remote visual/video Components, the local one
* will be displayed as if it is a remote one i.e. in the same grid, not
- * on top of a remote one.
+ * on top of a remote one. The same layout will be used when this
+ * instance is dedicated to a telephony conference.
*/
- if ((this.remotes.size() > 1) && (local != null))
+ if (conference || ((this.remotes.size() > 1) && (local != null)))
{
remotes = new ArrayList<Component>();
remotes.addAll(this.remotes);
- remotes.add(local);
+ if (local != null)
+ remotes.add(local);
}
else
remotes = this.remotes;
int remoteCount = remotes.size();
+ Dimension preferredLayoutSize;
- if (remoteCount == 0)
+ if (!conference && (remoteCount == 1))
{
/*
- * If there is no remote visual/video Component, the local one will
- * serve the preferredSize of the Container.
+ * If the videos are to be laid out as in a one-to-one call, the
+ * remote video has to fill the parent and the local video will be
+ * placed on top of the remote video. The remote video will be laid
+ * out now and the local video will be laid out later/further
+ * bellow.
*/
- if (local != null)
+ preferredLayoutSize = super.preferredLayoutSize(parent);
+ }
+ else if (remoteCount > 0)
+ {
+ int columns = calculateColumnCount(remotes);
+ int columnsMinus1 = columns - 1;
+ int rows = (remoteCount + columnsMinus1) / columns;
+ int i = 0;
+ Dimension[] preferredSizes = new Dimension[columns * rows];
+
+ for (Component remote : remotes)
{
- Dimension preferredSize = local.getPreferredSize();
+ int column = columnsMinus1 - (i % columns);
+ int row = i / columns;
- if (preferredSize != null)
- return preferredSize;
+ preferredSizes[column + row * columns]
+ = remote.getPreferredSize();
+
+ i++;
+ if (i >= remoteCount)
+ break;
}
- }
- else if (remoteCount == 1)
- {
- /*
- * If there is a single remote visual/video Component, the local one
- * will be on top of it so the remote one will serve the
- * preferredSize of the Container.
- */
- Dimension preferredSize = remotes.get(0).getPreferredSize();
- if (preferredSize != null)
- return preferredSize;
- }
- else if (remoteCount > 1)
- {
- int maxWidth = 0;
- int maxHeight = 0;
+ int preferredLayoutWidth = 0;
- for (Component remote : remotes)
+ for (int column = 0; column < columns; column++)
{
- Dimension preferredSize = remote.getPreferredSize();
+ int preferredColumnWidth = 0;
- if (preferredSize != null)
+ for (int row = 0; row < rows; row++)
{
- if (maxWidth < preferredSize.width)
- maxWidth = preferredSize.width;
- if (maxHeight < preferredSize.height)
- maxHeight = preferredSize.height;
+ Dimension preferredSize
+ = preferredSizes[column + row * columns];
+
+ if (preferredSize != null)
+ preferredColumnWidth += preferredSize.width;
}
+ preferredColumnWidth /= rows;
+
+ preferredLayoutWidth += preferredColumnWidth;
}
- if ((maxWidth > 0) && (maxHeight > 0))
+ int preferredLayoutHeight = 0;
+
+ for (int row = 0; row < rows; row++)
{
- int columns = calculateColumnCount(remotes);
- int rows = (remoteCount + columns - 1) / columns;
+ int preferredRowHeight = 0;
- return new Dimension(maxWidth * columns, maxHeight * rows);
+ for (int column = 0; column < columns; column++)
+ {
+ Dimension preferredSize
+ = preferredSizes[column + row * columns];
+
+ if (preferredSize != null)
+ preferredRowHeight = preferredSize.height;
+ }
+ preferredRowHeight /= columns;
+
+ preferredLayoutHeight += preferredRowHeight;
}
+
+ preferredLayoutSize
+ = new Dimension(
+ preferredLayoutWidth + columnsMinus1 * HGAP,
+ preferredLayoutHeight);
}
+ else
+ preferredLayoutSize = null;
- return super.preferredLayoutSize(parent);
+ if (local != null)
+ {
+ /*
+ * If the local visual/video Component is not displayed as if it is
+ * a remote one, it will be placed on top of a remote one. Then for
+ * the purposes of the preferredLayoutSize method it needs to be
+ * considered only if there is no remote video whatsoever.
+ */
+ if (!remotes.contains(local) && (preferredLayoutSize == null))
+ {
+ Dimension preferredSize = local.getPreferredSize();
+
+ if (preferredSize != null)
+ {
+ int preferredHeight
+ = Math.round(
+ preferredSize.height * LOCAL_TO_REMOTE_RATIO);
+ int preferredWidth
+ = Math.round(
+ preferredSize.width * LOCAL_TO_REMOTE_RATIO);
+
+ preferredLayoutSize
+ = new Dimension(preferredWidth, preferredHeight);
+ }
+ }
+
+ /*
+ * The closeButton has to be on top of the local video.
+ * Consequently, the preferredLayoutSize method does not have to
+ * consider it. Well, maybe if does if the local video is smaller
+ * than the closeButton... but that's just not cool anyway.
+ */
+ }
+
+ /*
+ * The video canvas will get the locations of the other components to
+ * paint so it has to cover the parent completely. In other words, the
+ * preferredLayoutSize method does not have to consider it.
+ */
+
+ return
+ (preferredLayoutSize == null)
+ ? super.preferredLayoutSize(parent)
+ : preferredLayoutSize;
}
/**
@@ -480,6 +590,11 @@ public void removeLayoutComponent(Component comp)
{
super.removeLayoutComponent(comp);
+ synchronized (constraints)
+ {
+ constraints.remove(comp);
+ }
+
if (local == comp)
local = null;
else if (closeButton == comp)
--
GitLab