From 76bd42da26fc73ecfe4955bbb1877f1a5012603a Mon Sep 17 00:00:00 2001 From: Thiago Camargo Date: Sat, 3 Mar 2007 03:48:59 +0000 Subject: [PATCH] Payload Negotiation Refactoring and bug fixes git-svn-id: http://svn.igniterealtime.org/svn/repos/smack/trunk@7362 b35dd754-fafc-0310-a699-88a17e54d16e --- .../jingle/media/JingleMediaManager.java | 5 - .../smackx/jingle/media/MediaNegotiator.java | 107 ++++++++++++------ .../smackx/packet/JingleError.java | 24 +++- .../smackx/jingle/JingleManagerTest.java | 2 +- .../jingleaudio/jmf/JmfMediaManager.java | 10 +- .../jingleaudio/jspeex/SpeexMediaManager.java | 9 +- jingle/media/test/JingleMediaTest.java | 13 ++- 7 files changed, 109 insertions(+), 61 deletions(-) diff --git a/jingle/extension/source/org/jivesoftware/smackx/jingle/media/JingleMediaManager.java b/jingle/extension/source/org/jivesoftware/smackx/jingle/media/JingleMediaManager.java index 06c90eb1c..c53ab1f1b 100644 --- a/jingle/extension/source/org/jivesoftware/smackx/jingle/media/JingleMediaManager.java +++ b/jingle/extension/source/org/jivesoftware/smackx/jingle/media/JingleMediaManager.java @@ -43,11 +43,6 @@ public abstract class JingleMediaManager { */ public abstract List getPayloads(); - /** - * Get the preferred Payload Type - */ - public abstract PayloadType getPreferredPayloadType(); - /** * Create a Media Session Implementation * diff --git a/jingle/extension/source/org/jivesoftware/smackx/jingle/media/MediaNegotiator.java b/jingle/extension/source/org/jivesoftware/smackx/jingle/media/MediaNegotiator.java index 8cd234a5e..7869ea1a4 100644 --- a/jingle/extension/source/org/jivesoftware/smackx/jingle/media/MediaNegotiator.java +++ b/jingle/extension/source/org/jivesoftware/smackx/jingle/media/MediaNegotiator.java @@ -1,3 +1,22 @@ +/** + * $RCSfile$ + * $Revision: 7329 $ + * $Date: 2007-02-28 20:59:28 -0300 (qua, 28 fev 2007) $ + * + * Copyright 2003-2005 Jive Software. + * + * All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.jivesoftware.smackx.jingle.media; import org.jivesoftware.smack.XMPPException; @@ -17,12 +36,13 @@ import java.util.List; /** * Manager for jmf descriptor negotiation. - * - * + *

+ *

* This class is responsible for managing the descriptor negotiation process, * handling all the xmpp packets interchange and the stage control. - * + * * @author Alvaro Saurin + * @author Thiago Camargo */ public class MediaNegotiator extends JingleNegotiator { @@ -90,31 +110,37 @@ public class MediaNegotiator extends JingleNegotiator { // With a null packet, we are just inviting the other end... setState(inviting); jout = getState().eventInvite(); - } else { + } + else { if (iq instanceof Jingle) { // If there is no specific jmf action associated, then we // are being invited to a new session... setState(accepting); jout = getState().eventInitiate((Jingle) iq); - } else { + } + else { throw new IllegalStateException( "Invitation IQ received is not a Jingle packet in Media negotiator."); } } - } else { + } + else { if (iq == null) { return null; - } else { + } + else { if (iq.getType().equals(IQ.Type.ERROR)) { // Process errors getState().eventError(iq); - } else if (iq.getType().equals(IQ.Type.RESULT)) { + } + else if (iq.getType().equals(IQ.Type.RESULT)) { // Process ACKs if (isExpectedId(iq.getPacketID())) { jout = getState().eventAck(iq); removeExpectedId(iq.getPacketID()); } - } else if (iq instanceof Jingle) { + } + else if (iq instanceof Jingle) { // Get the action from the Jingle packet Jingle jin = (Jingle) iq; Jingle.Action action = jin.getAction(); @@ -122,11 +148,14 @@ public class MediaNegotiator extends JingleNegotiator { if (action != null) { if (action.equals(Jingle.Action.CONTENTACCEPT)) { jout = getState().eventAccept(jin); - } else if (action.equals(Jingle.Action.CONTENTDECLINE)) { + } + else if (action.equals(Jingle.Action.CONTENTDECLINE)) { jout = getState().eventDecline(jin); - } else if (action.equals(Jingle.Action.DESCRIPTIONINFO)) { + } + else if (action.equals(Jingle.Action.DESCRIPTIONINFO)) { jout = getState().eventInfo(jin); - } else if (action.equals(Jingle.Action.CONTENTMODIFY)) { + } + else if (action.equals(Jingle.Action.CONTENTMODIFY)) { jout = getState().eventModify(jin); } // Any unknown action will be ignored: it is not a msg @@ -139,7 +168,8 @@ public class MediaNegotiator extends JingleNegotiator { // Save the Id for any ACK if (id != null) { addExpectedId(id); - } else { + } + else { if (jout != null) { addExpectedId(jout.getPacketID()); } @@ -169,8 +199,8 @@ public class MediaNegotiator extends JingleNegotiator { // Payload types private PayloadType.Audio calculateBestCommonAudioPt(List remoteAudioPts) { - final ArrayList commonAudioPtsHere = new ArrayList(); - final ArrayList commonAudioPtsThere = new ArrayList(); + final ArrayList commonAudioPtsHere = new ArrayList(); + final ArrayList commonAudioPtsThere = new ArrayList(); PayloadType.Audio result = null; if (!remoteAudioPts.isEmpty()) { @@ -181,26 +211,28 @@ public class MediaNegotiator extends JingleNegotiator { commonAudioPtsThere.retainAll(localAudioPts); if (!commonAudioPtsHere.isEmpty() && !commonAudioPtsThere.isEmpty()) { - PayloadType.Audio bestPtHere = (PayloadType.Audio) commonAudioPtsHere - .get(0); - PayloadType.Audio bestPtThere = (PayloadType.Audio) commonAudioPtsThere - .get(0); - // If both match, use it - if (bestPtHere.equals(bestPtThere)) { - result = bestPtHere; - } else { - // Otherwise, use the one of the initiator... - // FIXME: this is an invented behavior!!! - String initiator = session.getInitiator(); - String me = session.getConnection().getUser(); + PayloadType.Audio bestPtHere = null; + - if (initiator.equals(me)) { - result = bestPtHere; - } else { - result = bestPtThere; + if (bestPtHere == null) + for (PayloadType payloadType : commonAudioPtsHere) + if (payloadType instanceof PayloadType.Audio) { + bestPtHere = (PayloadType.Audio) payloadType; + break; + } + + PayloadType.Audio bestPtThere = null; + for (PayloadType payloadType : commonAudioPtsThere) + if (payloadType instanceof PayloadType.Audio) { + bestPtThere = (PayloadType.Audio) payloadType; + break; } - } + + if (session.getInitiator().equals(session.getConnection().getUser())) + result = bestPtHere; + else + result = bestPtThere; } } @@ -332,6 +364,7 @@ public class MediaNegotiator extends JingleNegotiator { * First stage when we send a session request. */ public class Inviting extends JingleNegotiator.State { + public Inviting(MediaNegotiator neg) { super(neg); } @@ -391,7 +424,8 @@ public class MediaNegotiator extends JingleNegotiator { // and send an accept if we havee an agreement... if (bestCommonAudioPt != null) { response = createAcceptMessage(); - } else { + } + else { throw new JingleException(JingleError.NO_COMMON_PAYLOAD); } @@ -407,6 +441,7 @@ public class MediaNegotiator extends JingleNegotiator { * accepts or not... */ public class Pending extends JingleNegotiator.State { + public Pending(MediaNegotiator neg) { super(neg); } @@ -441,7 +476,8 @@ public class MediaNegotiator extends JingleNegotiator { if (oldBestCommonAudioPt == null || ptChange) { response = createAcceptMessage(); } - } else { + } + else { throw new JingleException(JingleError.NO_COMMON_PAYLOAD); } } @@ -481,7 +517,8 @@ public class MediaNegotiator extends JingleNegotiator { } } - } else if (offeredPayloads.size() > 1) { + } + else if (offeredPayloads.size() > 1) { throw new JingleException(JingleError.MALFORMED_STANZA); } } diff --git a/jingle/extension/source/org/jivesoftware/smackx/packet/JingleError.java b/jingle/extension/source/org/jivesoftware/smackx/packet/JingleError.java index 3d9d37e88..7d92f76d7 100644 --- a/jingle/extension/source/org/jivesoftware/smackx/packet/JingleError.java +++ b/jingle/extension/source/org/jivesoftware/smackx/packet/JingleError.java @@ -1,3 +1,23 @@ +/** + * $RCSfile$ + * $Revision: 7329 $ + * $Date: 2007-02-28 20:59:28 -0300 (qua, 28 fev 2007) $ + * + * Copyright 2003-2005 Jive Software. + * + * All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.jivesoftware.smackx.packet; import org.jivesoftware.smack.packet.PacketExtension; @@ -19,7 +39,7 @@ public class JingleError implements PacketExtension { // Non standard error messages public static final JingleError NO_COMMON_PAYLOAD = new JingleError( - "no-common-payload"); + "unsupported-codecs"); public static final JingleError NEGOTIATION_ERROR = new JingleError( "negotiation-error"); @@ -76,7 +96,7 @@ public class JingleError implements PacketExtension { return UNSUPPORTED_CONTENT; } else if (value.equals("unsupported-transports")) { return UNSUPPORTED_TRANSPORTS; - } else if (value.equals("no-common-payload")) { + } else if (value.equals("unsupported-codecs")) { return NO_COMMON_PAYLOAD; } else if (value.equals("negotiation-error")) { return NEGOTIATION_ERROR; diff --git a/jingle/extension/test/org/jivesoftware/smackx/jingle/JingleManagerTest.java b/jingle/extension/test/org/jivesoftware/smackx/jingle/JingleManagerTest.java index 6cf3dd61f..526d417ef 100644 --- a/jingle/extension/test/org/jivesoftware/smackx/jingle/JingleManagerTest.java +++ b/jingle/extension/test/org/jivesoftware/smackx/jingle/JingleManagerTest.java @@ -849,7 +849,7 @@ public class JingleManagerTest extends SmackTestCase { return new ArrayList(); } - public PayloadType getPreferredPayloadType() { + public PayloadType.Audio getPreferredAudioPayloadType() { return null; } diff --git a/jingle/media/source/org/jivesoftware/jingleaudio/jmf/JmfMediaManager.java b/jingle/media/source/org/jivesoftware/jingleaudio/jmf/JmfMediaManager.java index d99913922..9168c1bc1 100644 --- a/jingle/media/source/org/jivesoftware/jingleaudio/jmf/JmfMediaManager.java +++ b/jingle/media/source/org/jivesoftware/jingleaudio/jmf/JmfMediaManager.java @@ -78,15 +78,7 @@ public class JmfMediaManager extends JingleMediaManager { public List getPayloads() { return payloads; } - - /** - * Get the preferred Payload Type - */ - public PayloadType getPreferredPayloadType() { - //TODO a better way to choose the preferred Payload - return payloads.size() > 0 ? payloads.get(0) : null; - } - + /** * Runs JMFInit the first time the application is started so that capture * devices are properly detected and initialized by JMF. diff --git a/jingle/media/source/org/jivesoftware/jingleaudio/jspeex/SpeexMediaManager.java b/jingle/media/source/org/jivesoftware/jingleaudio/jspeex/SpeexMediaManager.java index 4af896133..7b02fc406 100644 --- a/jingle/media/source/org/jivesoftware/jingleaudio/jspeex/SpeexMediaManager.java +++ b/jingle/media/source/org/jivesoftware/jingleaudio/jspeex/SpeexMediaManager.java @@ -65,14 +65,7 @@ public class SpeexMediaManager extends JingleMediaManager { public List getPayloads() { return payloads; } - - /** - * Get the preferred Payload Type - */ - public PayloadType getPreferredPayloadType() { - return payloads.size() > 0 ? payloads.get(0) : null; - } - + /** * Runs JMFInit the first time the application is started so that capture * devices are properly detected and initialized by JMF. diff --git a/jingle/media/test/JingleMediaTest.java b/jingle/media/test/JingleMediaTest.java index faac8f5bc..681cb40d1 100644 --- a/jingle/media/test/JingleMediaTest.java +++ b/jingle/media/test/JingleMediaTest.java @@ -15,6 +15,7 @@ import org.jivesoftware.smackx.jingle.media.JingleMediaManager; import org.jivesoftware.smackx.jingle.nat.BridgedTransportManager; import org.jivesoftware.smackx.jingle.nat.ICETransportManager; import org.jivesoftware.smackx.jingle.nat.STUNTransportManager; +import org.jivesoftware.smackx.jingle.nat.BasicTransportManager; import javax.media.MediaLocator; import javax.media.format.AudioFormat; @@ -109,6 +110,7 @@ public class JingleMediaTest extends SmackTestCase { XMPPConnection x0 = getConnection(0); XMPPConnection x1 = getConnection(1); +/* ICETransportManager icetm0 = new ICETransportManager(x0, "jivesoftware.com", 3478); ICETransportManager icetm1 = new ICETransportManager(x1, "jivesoftware.com", 3478); @@ -119,11 +121,20 @@ public class JingleMediaTest extends SmackTestCase { jm0.addCreationListener(icetm0); jm1.addCreationListener(icetm1); +*/ + + final JingleManager jm0 = new JingleManager( + x0, new BasicTransportManager()); + final JingleManager jm1 = new JingleManager( + x1, new BasicTransportManager()); MultiMediaManager jingleMediaManager0 = new MultiMediaManager(); jingleMediaManager0.addMediaManager(new SpeexMediaManager()); jingleMediaManager0.addMediaManager(new JmfMediaManager()); - JingleMediaManager jingleMediaManager1 = new JmfMediaManager(); + jingleMediaManager0.addMediaManager(new JmfMediaManager()); + MultiMediaManager jingleMediaManager1 = new MultiMediaManager(); + jingleMediaManager1.addMediaManager(new JmfMediaManager()); + jingleMediaManager1.addMediaManager(new SpeexMediaManager()); jm0.setMediaManager(jingleMediaManager0); jm1.setMediaManager(jingleMediaManager1);