[jitsi-dev] Re: [jitsi~svn:9377] Makes possible making video call without audio, and sharing the desktop f


#1

Hi Werner, Ingo,

I have changed the way zrtp handles master session, now when there is
one stream we select it as master, when there are more than one we
select the audio stream as master. Can you take a look, do you think
its ok?
This was made so we can have zrtp enabled on a video call without audio in it.

Regards
Damian

···

On Fri, Feb 17, 2012 at 12:19 PM, <damencho@java.net> wrote:

Project: jitsi
Repository: svn
Revision: 9377
Author: damencho
Date: 2012-02-17 10:19:07 UTC
Link:

Log Message:
------------
Makes possible making video call without audio, and sharing the desktop from machine without audio device.
Updates zrtp to use the first available stream as master, if more than one stream use the audio one.

Revisions:
----------
9377

Modified Paths:
---------------
trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControlImpl.java
trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java
trunk/src/net/java/sip/communicator/impl/neomedia/notify/PortAudioClipImpl.java
trunk/src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java
trunk/src/net/java/sip/communicator/service/protocol/media/CallPeerMediaHandler.java
trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java
trunk/src/net/java/sip/communicator/service/neomedia/SrtpControl.java
trunk/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java
trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java

Diffs:
------
Index: trunk/src/net/java/sip/communicator/service/protocol/media/CallPeerMediaHandler.java

--- trunk/src/net/java/sip/communicator/service/protocol/media/CallPeerMediaHandler.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/service/protocol/media/CallPeerMediaHandler.java (revision 9377)
@@ -1236,6 +1236,7 @@
* stream to use (i.e. sendonly, sendrecv, recvonly, or inactive).
* @param rtpExtensions the list of <tt>RTPExtension</tt>s that should be
* enabled for this stream.
+ * @param masterStream whether the stream to be used as master if secured
*
* @return the newly created <tt>MediaStream</tt>.
*
@@ -1247,7 +1248,8 @@
MediaFormat format,
MediaStreamTarget target,
MediaDirection direction,
- List<RTPExtension> rtpExtensions)
+ List<RTPExtension> rtpExtensions,
+ boolean masterStream)
throws OperationFailedException
{
MediaType mediaType = device.getMediaType();
@@ -1284,7 +1286,8 @@

    return
        configureStream\(

- device, format, target, direction, rtpExtensions, stream);
+ device, format, target, direction, rtpExtensions, stream,
+ masterStream);
}

/\*\*

@@ -1303,6 +1306,7 @@
* @param rtpExtensions the list of <tt>RTPExtension</tt>s that should be
* enabled for this stream.
* @param stream the <tt>MediaStream</tt> that we'd like to configure.
+ * @param masterStream whether the stream to be used as master if secured
*
* @return the <tt>MediaStream</tt> that we received as a parameter (for
* convenience reasons).
@@ -1316,7 +1320,8 @@
MediaStreamTarget target,
MediaDirection direction,
List<RTPExtension> rtpExtensions,
- MediaStream stream)
+ MediaStream stream,
+ boolean masterStream)
throws OperationFailedException
{
registerDynamicPTsWithStream(stream);
@@ -1355,8 +1360,9 @@
*/
SrtpControl srtpControl = stream.getSrtpControl();

+ srtpControl.setMasterSession(masterStream);
srtpControl.setSrtpListener(srtpListener);
- srtpControl.start(MediaType.AUDIO.equals(mediaType));
+ srtpControl.start(mediaType);
}

    return stream;

Index: trunk/src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java

--- trunk/src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java (revision 9377)
@@ -495,7 +495,10 @@
*/
if ((conferenceAudioMixer == null)
&& (device != null)
- && (!OSUtils.IS_ANDROID || isConferenceFocus()))
+ && (!OSUtils.IS_ANDROID || isConferenceFocus())
+ // we can use audio mixer only if we
+ // have capture device (device can send)
+ && (device.getDirection().allowsSending()))
conferenceAudioMixer = mediaService.createMixer(device);
if (conferenceAudioMixer != null)
device = conferenceAudioMixer;
Index: trunk/src/net/java/sip/communicator/service/neomedia/SrtpControl.java

--- trunk/src/net/java/sip/communicator/service/neomedia/SrtpControl.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/service/neomedia/SrtpControl.java (revision 9377)
@@ -47,11 +47,16 @@
public boolean getSecureCommunicationStatus();

/\*\*

+ * When in multistream mode, enables the master session.
+ * @param masterSession whether current control, controls the master session.
+ */
+ public void setMasterSession(boolean masterSession);
+
+ /**
* Starts and enables zrtp in the stream holding this control.
- * @param masterSession whether this stream is master for the current
- * media session.
+ * @param mediaType the media type of the stream this control controls.
*/
- public void start(boolean masterSession);
+ public void start(MediaType mediaType);

/\*\*
 \* Sets the multistream data, which means that the master stream

Index: trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java

--- trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java (revision 9377)
@@ -193,6 +193,7 @@
* stream to use (i.e. sendonly, sendrecv, recvonly, or inactive).
* @param rtpExtensions the list of <tt>RTPExtension</tt>s that should be
* enabled for this stream.
+ * @param masterStream whether the stream to be used as master if secured
*
* @return the newly created <tt>MediaStream</tt>.
*
@@ -205,7 +206,8 @@
MediaFormat format,
MediaStreamTarget target,
MediaDirection direction,
- List<RTPExtension> rtpExtensions)
+ List<RTPExtension> rtpExtensions,
+ boolean masterStream)
throws OperationFailedException
{
MediaStream stream
@@ -215,7 +217,8 @@
format,
target,
direction,
- rtpExtensions);
+ rtpExtensions,
+ masterStream);

    if\(stream \!= null\)
        stream\.setName\(streamName\);

@@ -457,10 +460,27 @@

    //user answered an incoming call so we go through whatever content
    //entries we are initializing and init their corresponding streams

+
+ // First parse content so we know how may streams,
+ // and what type of content we have
+ Map<ContentPacketExtension,
+ RtpDescriptionPacketExtension> contents
+ = new HashMap<ContentPacketExtension,
+ RtpDescriptionPacketExtension>();
for(ContentPacketExtension ourContent : sessAccept)
{
RtpDescriptionPacketExtension description
- = JingleUtils.getRtpDescription(ourContent);
+ = JingleUtils.getRtpDescription(ourContent);
+ contents.put(ourContent, description);
+ }
+
+ boolean masterStreamSet = false;
+ for(Map.Entry<ContentPacketExtension, RtpDescriptionPacketExtension> en
+ : contents.entrySet())
+ {
+ ContentPacketExtension ourContent = en.getKey();
+
+ RtpDescriptionPacketExtension description = en.getValue();
MediaType type = MediaType.parseString(description.getMedia());

        // stream connector

@@ -532,9 +552,28 @@
}
}

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the master
+ if(!masterStreamSet)
+ {
+ if(contents.size() > 1)
+ {
+ if(type.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+
// create the corresponding stream...
initStream(ourContent.getName(), connector, dev, format, target,
- direction, rtpExtensions);
+ direction, rtpExtensions, masterStream);

        // if remote peer requires inputevt, notify UI to capture mouse
        // and keyboard events

@@ -887,12 +926,37 @@
throws OperationFailedException,
IllegalArgumentException
{
+ boolean masterStreamSet = false;
for(String key : remoteContentMap.keySet())
{
ContentPacketExtension ext = remoteContentMap.get(key);

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the master
+ if(!masterStreamSet)
+ {
+ RtpDescriptionPacketExtension description
+ = JingleUtils.getRtpDescription(ext);
+ MediaType mediaType
+ = MediaType.parseString( description.getMedia() );
+
+ if(remoteContentMap.size() > 1)
+ {
+ if(mediaType.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+
if(ext != null)
- processContent(ext, false);
+ processContent(ext, false, masterStream);
}
}

@@ -924,13 +988,13 @@
{
if(modify)
{
- processContent(content, modify);
+ processContent(content, modify, false);
remoteContentMap.put(name, content);
}
else
{
ext.setSenders(content.getSenders());
- processContent(ext, modify);
+ processContent(ext, modify, false);
remoteContentMap.put(name, ext);
}
}
@@ -982,6 +1046,7 @@
*
* @param content a <tt>ContentPacketExtension</tt>
* @param modify if it correspond to a content-modify for resolution change
+ * @param masterStream whether the stream to be used as master
* @throws OperationFailedException if we fail to handle <tt>content</tt>
* for reasons like failing to initialize media devices or streams.
* @throws IllegalArgumentException if there's a problem with the syntax or
@@ -991,7 +1056,8 @@
* in this operation can synchronize to the mediaHandler instance to wait
* processing to stop (method setState in CallPeer).
*/
- private void processContent(ContentPacketExtension content, boolean modify)
+ private void processContent(ContentPacketExtension content, boolean modify,
+ boolean masterStream)
throws OperationFailedException,
IllegalArgumentException
{
@@ -1114,7 +1180,8 @@

    // create the corresponding stream\.\.\.
    initStream\(content\.getName\(\), connector, dev,

- supportedFormats.get(0), target, direction, rtpExtensions);
+ supportedFormats.get(0), target, direction, rtpExtensions,
+ masterStream);
}

/\*\*

@@ -1141,12 +1208,37 @@
* information compatible with that carried in transport-info.
*/
processTransportInfo(answer);
-
+
+ boolean masterStreamSet = false;
for (ContentPacketExtension content : answer)
{
remoteContentMap.put(content.getName(), content);

- processContent(content, false);
+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the master
+ if(!masterStreamSet)
+ {
+ RtpDescriptionPacketExtension description
+ = JingleUtils.getRtpDescription(content);
+ MediaType mediaType
+ = MediaType.parseString( description.getMedia() );
+
+ if(answer.size() > 1)
+ {
+ if(mediaType.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+
+ processContent(content, false, masterStream);
}
}

Index: trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java

--- trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java (revision 9377)
@@ -230,7 +230,8 @@
List<PayloadTypePacketExtension> lst = localContentMap.get("audio");

    description\.setNamespace\(SessionIQProvider\.GTALK\_AUDIO\_NAMESPACE\);

-
+
+ boolean masterStreamSet = false;
for(MediaType mediaType : MediaType.values())
{
MediaFormat format = null;
@@ -291,8 +292,27 @@
new ArrayList<RTPExtension>();
MediaDirection direction = MediaDirection.SENDRECV;

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the master
+ if(!masterStreamSet)
+ {
+ if(MediaType.values().length > 1)
+ {
+ if(mediaType.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+
initStream(mediaName, connector, dev, format, target,
- direction, rtpExtensions);
+ direction, rtpExtensions, masterStream);
}

    return description;

@@ -319,6 +339,7 @@
{
List<PayloadTypePacketExtension> lst = answer.getPayloadTypes();

+ boolean masterStreamSet = true;
for(MediaType mediaType : MediaType.values())
{
String ns = getNamespaceForMediaType(mediaType);
@@ -360,8 +381,27 @@
List<RTPExtension> rtpExtensions = new ArrayList<RTPExtension>();
MediaDirection direction = MediaDirection.SENDRECV;

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the master
+ if(!masterStreamSet)
+ {
+ if(MediaType.values().length > 1)
+ {
+ if(mediaType.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+
initStream(mediaName, connector, dev, format, target,
- direction, rtpExtensions);
+ direction, rtpExtensions, masterStream);
}
}

@@ -565,8 +605,7 @@
* Create list of payload types for device.
*
* @param supportedFormats supported formats of a device
- * @param direction direction
- * @param supportedExtensions supported RTP extensions
+ * @param name name of payload type
* @return list of payload types for this device
*/
private List<PayloadTypePacketExtension> createPayloadTypesForOffer(
@@ -663,6 +702,7 @@
* stream to use (i.e. sendonly, sendrecv, recvonly, or inactive).
* @param rtpExtensions the list of <tt>RTPExtension</tt>s that should be
* enabled for this stream.
+ * @param masterStream whether the stream to be used as master if secured
*
* @return the newly created <tt>MediaStream</tt>.
*
@@ -675,7 +715,8 @@
MediaFormat format,
MediaStreamTarget target,
MediaDirection direction,
- List<RTPExtension> rtpExtensions)
+ List<RTPExtension> rtpExtensions,
+ boolean masterStream)
throws OperationFailedException
{
if(format instanceof VideoMediaFormat)
@@ -696,7 +737,8 @@
format,
target,
direction,
- rtpExtensions);
+ rtpExtensions,
+ masterStream);

    if\(stream \!= null\)
        stream\.setName\(streamName\);

Index: trunk/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java

--- trunk/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java (revision 9377)
@@ -415,6 +415,7 @@
.getAccountPropertyInt(ProtocolProviderFactory.SAVP_OPTION,
ProtocolProviderFactory.SAVP_OFF);

+ boolean masterStreamSet = false;
List<MediaType> seenMediaTypes = new ArrayList<MediaType>();
for (MediaDescription mediaDescription : remoteDescriptions)
{
@@ -590,8 +591,29 @@
// create the corresponding stream...
MediaFormat fmt = findMediaFormat(remoteFormats,
mutuallySupportedFormats.get(0));
- initStream(connector, dev, fmt, target, direction, rtpExtensions);

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the master
+ if(!masterStreamSet)
+ {
+ if(remoteDescriptions.size() > 1)
+ {
+ if(mediaType.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+
+ initStream(connector, dev, fmt, target, direction, rtpExtensions,
+ masterStream);
+
// create the answer description
answerDescriptions.add(md);

@@ -847,6 +869,7 @@

    this\.setCallInfoURL\(SdpUtils\.getCallInfoURL\(answer\)\);

+ boolean masterStreamSet = false;
List<MediaType> seenMediaTypes = new ArrayList<MediaType>();
for (MediaDescription mediaDescription : remoteDescriptions)
{
@@ -992,9 +1015,28 @@
}
}

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the master
+ if(!masterStreamSet)
+ {
+ if(remoteDescriptions.size() > 1)
+ {
+ if(mediaType.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+
// create the corresponding stream...
initStream(connector, dev, supportedFormats.get(0), target,
- direction, rtpExtensions);
+ direction, rtpExtensions, masterStream);
}
}

Index: trunk/src/net/java/sip/communicator/impl/neomedia/notify/PortAudioClipImpl.java

--- trunk/src/net/java/sip/communicator/impl/neomedia/notify/PortAudioClipImpl.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/notify/PortAudioClipImpl.java (revision 9377)
@@ -193,12 +193,16 @@
* If the user has configured PortAudio to use no notification device,
* don't try to play this clip.
*/
- MediaLocator rendererLocator
+ CaptureDeviceInfo audioNotifyDeviceInfo
= audioNotifier
- .getDeviceConfiguration().getAudioNotifyDevice().getLocator();
+ .getDeviceConfiguration().getAudioNotifyDevice();
+ if(audioNotifyDeviceInfo == null)
+ return false;

+ MediaLocator rendererLocator = audioNotifyDeviceInfo.getLocator();
if (rendererLocator == null)
return false;
+
renderer.setLocator(rendererLocator);

    AudioInputStream audioStream = null;

Index: trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControlImpl.java

--- trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControlImpl.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControlImpl.java (revision 9377)
@@ -14,9 +14,9 @@

import net.java.sip.communicator.impl.neomedia.*;
import net.java.sip.communicator.impl.neomedia.transform.*;
+import net.java.sip.communicator.impl.neomedia.transform.zrtp.*;
import net.java.sip.communicator.service.neomedia.*;
import net.java.sip.communicator.service.neomedia.event.*;
-import net.java.sip.communicator.service.protocol.event.*;

/**
* Default implementation of {@link SDesControl} that supports the crypto suites
@@ -93,12 +93,19 @@
return engine != null;
}

- public void start(boolean masterSession)
+ /**
+ * Not used.
+ * @param masterSession not used.
+ */
+ public void setMasterSession(boolean masterSession)
+ {}
+
+ public void start(MediaType type)
{
srtpListener.securityTurnedOn(
- masterSession ?
- CallPeerSecurityStatusEvent.AUDIO_SESSION :
- CallPeerSecurityStatusEvent.VIDEO_SESSION,
+ type.equals(MediaType.AUDIO) ?
+ SecurityEventManager.AUDIO_SESSION
+ : SecurityEventManager.VIDEO_SESSION,
selectedInAttribute.getCryptoSuite().encode(), this);
}

Index: trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java

--- trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java (revision 9376)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java (revision 9377)
@@ -52,6 +52,11 @@
private AbstractRTPConnector zrtpConnector = null;

/\*\*

+ * Whether current is master session.
+ */
+ private boolean masterSession = false;
+
+ /**
* Creates the control.
*/
ZrtpControlImpl()
@@ -139,15 +144,26 @@
}

/\*\*

+ * When in multistream mode, enables the master session.
+ * @param masterSession whether current control, controls the master session.
+ */
+ public void setMasterSession(boolean masterSession)
+ {
+ // by default its not master, change only if set to be master
+ // sometimes (jingle) streams are re-initing and
+ // we must reuse old value (true) event that false is submitted
+ if(masterSession)
+ this.masterSession = masterSession;
+ }
+
+ /**
* Starts and enables zrtp in the stream holding this control.
- * @param masterSession whether this stream is master for the current
- * media session.
+ * @param mediaType the media type of the stream this control controls.
*/
- public void start(boolean masterSession)
+ public void start(MediaType mediaType)
{
+ boolean zrtpAutoStart;

- boolean zrtpAutoStart = false;
-
// ZRTP engine initialization
ZRTPTransformEngine engine = getTransformEngine();
// Create security user callback for each peer.
@@ -170,7 +186,10 @@

        // we know that audio is considered as master for zrtp
        securityEventManager\.setSessionType\(

- SecurityEventManager.AUDIO_SESSION);
+ mediaType.equals(MediaType.AUDIO) ?
+ SecurityEventManager.AUDIO_SESSION
+ : SecurityEventManager.VIDEO_SESSION
+ );
}
else
{
@@ -180,7 +199,9 @@
// initially engine has value enableZrtp = false
zrtpAutoStart = zrtpEngine.isEnableZrtp();
securityEventManager.setSessionType(
- SecurityEventManager.VIDEO_SESSION);
+ mediaType.equals(MediaType.AUDIO) ?
+ SecurityEventManager.AUDIO_SESSION
+ : SecurityEventManager.VIDEO_SESSION);
}

    // tells the engine whether to autostart\(enable\)

#2

Hey

I have changed the way zrtp handles master session, now when there is
one stream we select it as master, when there are more than one we
select the audio stream as master. Can you take a look, do you think its
ok? This was made so we can have zrtp enabled on a video call without
audio in it.

Generally, I think your changes are okay.

For ZRTP, there's still the issue that media data needs to flow in both
directions to enable it at all. So if you start a video-only call where only
one participant sends video (or audio, for the matter), ZRTP will never be
enabled. But that's not really related to your changes.

Btw: How are you supposed to check the SAS with video only? Write it on a
paper and hold it to the webcam? :slight_smile:

There are some comments to the code inline.

Regards
Damian

Regards,
Ingo

Revisions:
----------
9377

Index:

trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandle
rJabberImpl.java

===================================================================
@@ -532,9 +552,28 @@
                }
            }

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the

master

+ if(!masterStreamSet)
+ {
+ if(contents.size() > 1)
+ {
+ if(type.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }

This if-set appears in a very similar way in at least half a dozen
locations. Couldn't that somehow be extracted into a distinct method? I'd
prefer that even if it would cause multiple iterations of
content-descriptions/MediaTypes/whatever. Not that it's wrong how it is, but
I'm feeling awkward...

Index:

trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControl
Impl.java

===================================================================
---

trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControl
Impl.java (revision 9376)

+++

trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControl
Impl.java (revision 9377)

@@ -14,9 +14,9 @@

import net.java.sip.communicator.impl.neomedia.*;
import net.java.sip.communicator.impl.neomedia.transform.*;
+import net.java.sip.communicator.impl.neomedia.transform.zrtp.*;

That import should not be here...

@@ -93,12 +93,19 @@
        return engine != null;
    }

- public void start(boolean masterSession)
+ /**
+ * Not used.
+ * @param masterSession not used.
+ */
+ public void setMasterSession(boolean masterSession)
+ {}
+
+ public void start(MediaType type)
    {
        srtpListener.securityTurnedOn(
- masterSession ?
- CallPeerSecurityStatusEvent.AUDIO_SESSION :
- CallPeerSecurityStatusEvent.VIDEO_SESSION,
+ type.equals(MediaType.AUDIO) ?
+ SecurityEventManager.AUDIO_SESSION
+ : SecurityEventManager.VIDEO_SESSION,
            selectedInAttribute.getCryptoSuite().encode(), this);

Note to myself: Get rid of those X_SESSION constants and use the MediaType
enum instead.

    }

Index:

trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java

===================================================================
--- trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java

(revision 9376)

+++ trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java

(revision 9377)

@@ -52,6 +52,11 @@
    private AbstractRTPConnector zrtpConnector = null;

    /**
+ * Whether current is master session.
+ */
+ private boolean masterSession = false;
+
+ /**
     * Creates the control.
     */
    ZrtpControlImpl()
@@ -139,15 +144,26 @@
    }

    /**
+ * When in multistream mode, enables the master session.
+ * @param masterSession whether current control, controls the master

session.

+ */
+ public void setMasterSession(boolean masterSession)
+ {
+ // by default its not master, change only if set to be master
+ // sometimes (jingle) streams are re-initing and
+ // we must reuse old value (true) event that false is submitted
+ if(masterSession)

This feels like a hack in newly created code. I guess this comes from
"processContent(content, modify, false);"? Couldn't this be handled there
correctly?

···

+ this.masterSession = masterSession;
+ }


#3

Hey

I have changed the way zrtp handles master session, now when there is
one stream we select it as master, when there are more than one we
select the audio stream as master. Can you take a look, do you think its
ok? This was made so we can have zrtp enabled on a video call without
audio in it.

Generally, I think your changes are okay.

For ZRTP, there's still the issue that media data needs to flow in both
directions to enable it at all. So if you start a video-only call where only
one participant sends video (or audio, for the matter), ZRTP will never be
enabled. But that's not really related to your changes.

Btw: How are you supposed to check the SAS with video only? Write it on a
paper and hold it to the webcam? :slight_smile:

You could also rely on the confirmation having been made earlier.

Emil

···

On 17.02.12 22:03, Ingo Bauersachs wrote:

There are some comments to the code inline.

Regards
Damian

Regards,
Ingo

Revisions:
----------
9377

Index:

trunk/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandle
rJabberImpl.java

===================================================================
@@ -532,9 +552,28 @@
                }
            }

+ boolean masterStream = false;
+ // if we have more than one stream, lets the audio be the

master

+ if(!masterStreamSet)
+ {
+ if(contents.size() > 1)
+ {
+ if(type.equals(MediaType.AUDIO))
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }
+ else
+ {
+ masterStream = true;
+ masterStreamSet = true;
+ }
+ }

This if-set appears in a very similar way in at least half a dozen
locations. Couldn't that somehow be extracted into a distinct method? I'd
prefer that even if it would cause multiple iterations of
content-descriptions/MediaTypes/whatever. Not that it's wrong how it is, but
I'm feeling awkward...

Index:

trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControl
Impl.java

===================================================================
---

trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControl
Impl.java (revision 9376)

+++

trunk/src/net/java/sip/communicator/impl/neomedia/transform/sdes/SDesControl
Impl.java (revision 9377)

@@ -14,9 +14,9 @@

import net.java.sip.communicator.impl.neomedia.*;
import net.java.sip.communicator.impl.neomedia.transform.*;
+import net.java.sip.communicator.impl.neomedia.transform.zrtp.*;

That import should not be here...

@@ -93,12 +93,19 @@
        return engine != null;
    }

- public void start(boolean masterSession)
+ /**
+ * Not used.
+ * @param masterSession not used.
+ */
+ public void setMasterSession(boolean masterSession)
+ {}
+
+ public void start(MediaType type)
    {
        srtpListener.securityTurnedOn(
- masterSession ?
- CallPeerSecurityStatusEvent.AUDIO_SESSION :
- CallPeerSecurityStatusEvent.VIDEO_SESSION,
+ type.equals(MediaType.AUDIO) ?
+ SecurityEventManager.AUDIO_SESSION
+ : SecurityEventManager.VIDEO_SESSION,
            selectedInAttribute.getCryptoSuite().encode(), this);

Note to myself: Get rid of those X_SESSION constants and use the MediaType
enum instead.

    }

Index:

trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java

===================================================================
--- trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java

(revision 9376)

+++ trunk/src/net/java/sip/communicator/impl/neomedia/ZrtpControlImpl.java

(revision 9377)

@@ -52,6 +52,11 @@
    private AbstractRTPConnector zrtpConnector = null;

    /**
+ * Whether current is master session.
+ */
+ private boolean masterSession = false;
+
+ /**
     * Creates the control.
     */
    ZrtpControlImpl()
@@ -139,15 +144,26 @@
    }

    /**
+ * When in multistream mode, enables the master session.
+ * @param masterSession whether current control, controls the master

session.

+ */
+ public void setMasterSession(boolean masterSession)
+ {
+ // by default its not master, change only if set to be master
+ // sometimes (jingle) streams are re-initing and
+ // we must reuse old value (true) event that false is submitted
+ if(masterSession)

This feels like a hack in newly created code. I guess this comes from
"processContent(content, modify, false);"? Couldn't this be handled there
correctly?

+ this.masterSession = masterSession;
+ }

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
Jitsi
emcho@jitsi.org PHONE: +33.1.77.62.43.30
http://jitsi.org FAX: +33.1.77.62.47.31