[sip-comm-dev] Re: svn commit: r6625 - trunk/src/net/java/sip/communicator/impl/neomedia: device imgstreaming protocol


#1

Hey Seb,

I haven't checked it but this mute support fix seems overly simple to
me. Does it actually work for capture devices for which the local
video support is enabled? The fix will currently get a mute-able
capture device, wrap it into a cloneable and then I'm not sure the
checks for mute support on the cloneable will succeed.

Regards,
Lubo

···

On Sat, Jan 16, 2010 at 12:32 PM, <s_vincent@dev.java.net> wrote:

Author: s_vincent
Date: 2010-01-16 10:32:47+0000
New Revision: 6625

Modified:
trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java
trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java
trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java
trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java
trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java

Log:
- Enable again muting support for video DataSource;
- Add getLocator() method to Push|PullDataSourceDelegate (otherwise MutePush|PullDataSource classes return null and we cannot check if locator is QT/Desktop streaming to disable local video);
- Disable local video for desktop streaming.

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java 2010-01-16 10:32:47+0000
@@ -27,7 +27,7 @@
{
String name = "Desktop streaming";
CaptureDeviceInfo devInfo = new CaptureDeviceInfo(name,
- new MediaLocator(ImageStreamingUtils.LOCATOR_PREFIX + name),
+ new MediaLocator(ImageStreamingUtils.LOCATOR_PREFIX + ":" + name),
DataSource.getFormats());

    /\* add to JMF device manager \*/

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java 2010-01-16 10:32:47+0000
@@ -12,6 +12,8 @@
import javax.media.protocol.*;

import net.java.sip.communicator.impl.neomedia.*;
+import net.java.sip.communicator.impl.neomedia.protocol.*;
+import net.java.sip.communicator.impl.neomedia.imgstreaming.*;
import net.java.sip.communicator.service.neomedia.*;
import net.java.sip.communicator.service.neomedia.event.*;
import net.java.sip.communicator.util.*;
@@ -90,23 +92,22 @@
* Create our DataSource as SourceCloneable so we can use it to both
* display local video and stream to remote peer.
*/
- /*
- * FIXME By overriding the super implementation, we have disabled muting
- * support.
- */
- DataSource captureDevice = getDevice().createOutputDataSource();
+ DataSource captureDevice = super.createCaptureDevice();

    if \(captureDevice \!= null\)
    \{
        MediaLocator locator = captureDevice\.getLocator\(\);

-
+
/*
* FIXME There is no video in calls when using the QuickTime/QTKit
- * CaptureDevice so the local video support is disabled for it now.
+ * CaptureDevice and desktop streaming, so the local video support
+ * is disabled for them now.
*/
if ((locator == null)
- || !QuickTimeAuto.LOCATOR_PROTOCOL
- .equals(locator.getProtocol()))
+ || (!QuickTimeAuto.LOCATOR_PROTOCOL
+ .equals(locator.getProtocol()) &&
+ !ImageStreamingUtils.LOCATOR_PREFIX
+ .equals(locator.getProtocol())))
{
DataSource cloneableDataSource
= Manager.createCloneableDataSource(captureDevice);

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java 2010-01-16 10:32:47+0000
@@ -19,7 +19,7 @@
/**
* The locator prefix used when creating or parsing <tt>MediaLocator</tt>s.
*/
- public static final String LOCATOR_PREFIX = "imgstreaming:";
+ public static final String LOCATOR_PREFIX = "imgstreaming";

/\*\*
 \* Get a scaled &lt;tt&gt;BufferedImage&lt;/tt&gt;\.

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java 2010-01-16 10:32:47+0000
@@ -97,6 +97,19 @@
}

/\*\*

+ * Implements {@link DataSource#getLocator()}. Delegates to the wrapped
+ * <tt>DataSource</tt>.
+ *
+ * @return a <tt>MediaLocator</tt> value which describes the locator of the
+ * wrapped <tt>DataSource</tt>
+ */
+ @Override
+ public MediaLocator getLocator()
+ {
+ return dataSource.getLocator();
+ }
+
+ /**
* Implements {@link DataSource#getControl(String)}. Delegates to the
* wrapped <tt>DataSource</tt>. Overrides
* {@link CaptureDeviceDelegatePullBufferDataSource#getControl(String)}

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java 2010-01-16 10:32:47+0000
@@ -97,6 +97,19 @@
}

/\*\*

+ * Implements {@link DataSource#getLocator()}. Delegates to the wrapped
+ * <tt>DataSource</tt>.
+ *
+ * @return a <tt>MediaLocator</tt> value which describes the locator of the
+ * wrapped <tt>DataSource</tt>
+ */
+ @Override
+ public MediaLocator getLocator()
+ {
+ return dataSource.getLocator();
+ }
+
+ /**
* Implements {@link DataSource#getControl(String)}. Delegates to the
* wrapped <tt>DataSource</tt>. Overrides
* {@link CaptureDeviceDelegatePushBufferDataSource#getControl(String)}

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: commits-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Hi,

Ouch, yes you're right. In fact if we createClone we lose MutePush>PullBufferDataSource capatiblity and if we create a MutePush>PullDataSource from a cloneable we lose SourceCloneable capability.

So we can create a "mute" video DataSource for sending but we will not have local video or we can create a standard video DataSource and have a "mute" local video DataSource.

I am not sure but maybe MutePush|PullBufferDataSource classes should implement SourceCloneable. WDYT ?

···

--
Seb

Lubomir Marinov a �crit :

Hey Seb,

I haven't checked it but this mute support fix seems overly simple to
me. Does it actually work for capture devices for which the local
video support is enabled? The fix will currently get a mute-able
capture device, wrap it into a cloneable and then I'm not sure the
checks for mute support on the cloneable will succeed.

Regards,
Lubo

On Sat, Jan 16, 2010 at 12:32 PM, <s_vincent@dev.java.net> wrote:
  

Author: s_vincent
Date: 2010-01-16 10:32:47+0000
New Revision: 6625

Modified:
  trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java
  trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java
  trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java
  trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java
  trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java

Log:
- Enable again muting support for video DataSource;
- Add getLocator() method to Push|PullDataSourceDelegate (otherwise MutePush|PullDataSource classes return null and we cannot check if locator is QT/Desktop streaming to disable local video);
- Disable local video for desktop streaming.

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/device/ImageStreamingAuto.java 2010-01-16 10:32:47+0000
@@ -27,7 +27,7 @@
    {
        String name = "Desktop streaming";
        CaptureDeviceInfo devInfo = new CaptureDeviceInfo(name,
- new MediaLocator(ImageStreamingUtils.LOCATOR_PREFIX + name),
+ new MediaLocator(ImageStreamingUtils.LOCATOR_PREFIX + ":" + name),
            DataSource.getFormats());

        /* add to JMF device manager */

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/device/VideoMediaDeviceSession.java 2010-01-16 10:32:47+0000
@@ -12,6 +12,8 @@
import javax.media.protocol.*;

import net.java.sip.communicator.impl.neomedia.*;
+import net.java.sip.communicator.impl.neomedia.protocol.*;
+import net.java.sip.communicator.impl.neomedia.imgstreaming.*;
import net.java.sip.communicator.service.neomedia.*;
import net.java.sip.communicator.service.neomedia.event.*;
import net.java.sip.communicator.util.*;
@@ -90,23 +92,22 @@
         * Create our DataSource as SourceCloneable so we can use it to both
         * display local video and stream to remote peer.
         */
- /*
- * FIXME By overriding the super implementation, we have disabled muting
- * support.
- */
- DataSource captureDevice = getDevice().createOutputDataSource();
+ DataSource captureDevice = super.createCaptureDevice();

        if (captureDevice != null)
        {
            MediaLocator locator = captureDevice.getLocator();
-
+
            /*
             * FIXME There is no video in calls when using the QuickTime/QTKit
- * CaptureDevice so the local video support is disabled for it now.
+ * CaptureDevice and desktop streaming, so the local video support
+ * is disabled for them now.
             */
            if ((locator == null)
- || !QuickTimeAuto.LOCATOR_PROTOCOL
- .equals(locator.getProtocol()))
+ || (!QuickTimeAuto.LOCATOR_PROTOCOL
+ .equals(locator.getProtocol()) &&
+ !ImageStreamingUtils.LOCATOR_PREFIX
+ .equals(locator.getProtocol())))
            {
                DataSource cloneableDataSource
                    = Manager.createCloneableDataSource(captureDevice);

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/imgstreaming/ImageStreamingUtils.java 2010-01-16 10:32:47+0000
@@ -19,7 +19,7 @@
    /**
     * The locator prefix used when creating or parsing <tt>MediaLocator</tt>s.
     */
- public static final String LOCATOR_PREFIX = "imgstreaming:";
+ public static final String LOCATOR_PREFIX = "imgstreaming";

    /**
     * Get a scaled <tt>BufferedImage</tt>.

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PullBufferDataSourceDelegate.java 2010-01-16 10:32:47+0000
@@ -97,6 +97,19 @@
    }

    /**
+ * Implements {@link DataSource#getLocator()}. Delegates to the wrapped
+ * <tt>DataSource</tt>.
+ *
+ * @return a <tt>MediaLocator</tt> value which describes the locator of the
+ * wrapped <tt>DataSource</tt>
+ */
+ @Override
+ public MediaLocator getLocator()
+ {
+ return dataSource.getLocator();
+ }
+
+ /**
     * Implements {@link DataSource#getControl(String)}. Delegates to the
     * wrapped <tt>DataSource</tt>. Overrides
     * {@link CaptureDeviceDelegatePullBufferDataSource#getControl(String)}

Modified: trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java?view=diff&rev=6625&p1=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java&p2=trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java&r1=6624&r2=6625

--- trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java (original)
+++ trunk/src/net/java/sip/communicator/impl/neomedia/protocol/PushBufferDataSourceDelegate.java 2010-01-16 10:32:47+0000
@@ -97,6 +97,19 @@
    }

    /**
+ * Implements {@link DataSource#getLocator()}. Delegates to the wrapped
+ * <tt>DataSource</tt>.
+ *
+ * @return a <tt>MediaLocator</tt> value which describes the locator of the
+ * wrapped <tt>DataSource</tt>
+ */
+ @Override
+ public MediaLocator getLocator()
+ {
+ return dataSource.getLocator();
+ }
+
+ /**
     * Implements {@link DataSource#getControl(String)}. Delegates to the
     * wrapped <tt>DataSource</tt>. Overrides
     * {@link CaptureDeviceDelegatePushBufferDataSource#getControl(String)}

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: commits-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hi Seb,

I am not sure but maybe MutePush|PullBufferDataSource classes should
implement SourceCloneable. WDYT ?

I guess that's how we're going to implement it eventually. And it will
mean that when we go mute, the local video will also go mute which is
probably what we want.

There's still something I'm not clear about though. SourceCloneable is
the interface through which one creates a clone and one is supposed to
get a DataSource which implements this interface through
Manager.createCloneableDataSource(DataSource) . The latter would
suggest that given whatever DataSource, JMF will go through fire to
derive a SourceCloneable DataSource even if it means wrapping. And
then if one already has a DataSource which implements SourceCloneable
and createCloneableDataSource doesn't work for DataSources which don't
implement SourceCloneable, why would one need
createCloneableDataSource in the first place when casting the
DataSource would be the only thing to do?

Regards,
Lubo

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

There's still something I'm not clear about though. SourceCloneable is
the interface through which one creates a clone and one is supposed to
get a DataSource which implements this interface through
Manager.createCloneableDataSource(DataSource) . The latter would
suggest that given whatever DataSource, JMF will go through fire to
derive a SourceCloneable DataSource even if it means wrapping. And
then if one already has a DataSource which implements SourceCloneable
and createCloneableDataSource doesn't work for DataSources which don't
implement SourceCloneable, why would one need
createCloneableDataSource in the first place when casting the
DataSource would be the only thing to do?

JMF indeed has a myriad of internal classes in order to enable
cleateCloneableDataSource to provide SourceCloneable to arbitrary
DataSources.

The error on my side which broke the correct functioning of the
SourceCloneable created for AbstractPushBufferCaptureDevice (i.e. the
imgstreaming and QTKit CaptureDevices) was that I disposed of the
PushBufferStreams upon disconnecting the DataSource. One of the
internal classes, namely SuperCloneableDataSource, turned out to
request the streams of the DataSource without connecting to it and to
assume that they don't change after disconnects. The Javadocs don't
mention anything on the subject of when calls to
PushBufferDataSource/PullBufferDataSource/PushDataSource/PullDataSource#getStreams()
are legal and only states that the support for some operations on
DataSources may be determined only after connecting.

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net