From f9b19f51227860de1634783d362b27da39c38025 Mon Sep 17 00:00:00 2001
From: Lyubomir Marinov <lyubomir.marinov@jitsi.org>
Date: Tue, 3 Nov 2015 17:14:20 -0600
Subject: [PATCH] Works around a failure to detect ReceiveStreams. Improves the
 implementations of the Min and Max ThroughputRTCPTerminationStrategies.

---
 .../BasicRTCPTerminationStrategy.java         | 79 ++++++++++++++-----
 .../MaxThroughputRTCPTerminationStrategy.java | 19 ++---
 .../MinThroughputRTCPTerminationStrategy.java | 19 ++---
 3 files changed, 79 insertions(+), 38 deletions(-)

diff --git a/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/BasicRTCPTerminationStrategy.java b/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/BasicRTCPTerminationStrategy.java
index 3ed5891f..9bb44e03 100644
--- a/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/BasicRTCPTerminationStrategy.java
+++ b/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/BasicRTCPTerminationStrategy.java
@@ -38,6 +38,7 @@
  * information that it collects and from information found in FMJ.
  *
  * @author George Politis
+ * @author Lyubomir Marinov
  */
 public class BasicRTCPTerminationStrategy
     extends MediaStreamRTCPTerminationStrategy
@@ -408,13 +409,27 @@ private RTCPReportBlock[] makeRTCPReportBlocks(long time)
             return MIN_RTCP_REPORTS_BLOCKS_ARRAY;
         }
 
-        Collection<ReceiveStream> receiveStreams
-            = streamRTPManager.getReceiveStreams();
+        Collection<ReceiveStream> receiveStreams;
 
-        if (receiveStreams == null || receiveStreams.size() == 0)
+        // XXX MediaStreamImpl's implementation of #getReceiveStreams() says
+        // that, unfortunately, it has been observed that sometimes there are
+        // valid ReceiveStreams in MediaStreamImpl which are not returned by
+        // FMJ's RTPManager. Since (1) MediaStreamImpl#getReceiveStreams() will
+        // include the results of StreamRTPManager#getReceiveStreams() and (2)
+        // we are going to check the results against SSRCCache, it should be
+        // relatively safe to rely on MediaStreamImpl's implementation.
+        if (stream instanceof MediaStreamImpl)
         {
-            logger.info("There are no receive streams to build report " +
-                "blocks for.");
+            receiveStreams = ((MediaStreamImpl) stream).getReceiveStreams();
+        }
+        else
+        {
+            receiveStreams = streamRTPManager.getReceiveStreams();
+        }
+        if (receiveStreams == null || receiveStreams.isEmpty())
+        {
+            logger.info(
+                    "There are no receive streams to build report blocks for.");
             return MIN_RTCP_REPORTS_BLOCKS_ARRAY;
         }
 
@@ -425,11 +440,10 @@ private RTCPReportBlock[] makeRTCPReportBlocks(long time)
             return MIN_RTCP_REPORTS_BLOCKS_ARRAY;
         }
 
-        // Create the return object.
+        // Create and populate the return object.
         Collection<RTCPReportBlock> rtcpReportBlocks
             = new ArrayList<RTCPReportBlock>();
 
-        // Populate the return object.
         for (ReceiveStream receiveStream : receiveStreams)
         {
             // Dig into the guts of FMJ and get the stats for the current
@@ -443,8 +457,9 @@ private RTCPReportBlock[] makeRTCPReportBlocks(long time)
             }
         }
 
-        return rtcpReportBlocks.toArray(
-            new RTCPReportBlock[rtcpReportBlocks.size()]);
+        return
+            rtcpReportBlocks.toArray(
+                    new RTCPReportBlock[rtcpReportBlocks.size()]);
     }
 
     /**
@@ -454,7 +469,7 @@ private RTCPReportBlock[] makeRTCPReportBlocks(long time)
      * @return an <tt>RTCPREMBPacket</tt> that provides receiver feedback to the
      * endpoint from which we receive.
      */
-    protected RTCPREMBPacket makeRTCPREMBPacket()
+    private RTCPREMBPacket makeRTCPREMBPacket()
     {
         // TODO we should only make REMBs if REMB support has been advertised.
         // Destination
@@ -472,22 +487,46 @@ protected RTCPREMBPacket makeRTCPREMBPacket()
         for (Integer ssrc : ssrcs)
             dest[i++] = ssrc & 0xFFFFFFFFL;
 
-        // Exp & mantissa
-        long bitrate = remoteBitrateEstimator.getLatestEstimate();
-
-        if (bitrate == -1)
-            return null;
-
-        if (logger.isDebugEnabled())
-            logger.debug("Estimated bitrate: " + bitrate);
-
         // Create and return the packet.
         // We use the stream's local source ID (SSRC) as the SSRC of packet
         // sender.
         long streamSSRC = getLocalSSRC();
 
         return
-            new RTCPREMBPacket(streamSSRC, /* mediaSSRC */ 0L, bitrate, dest);
+                makeRTCPREMBPacket(
+                        remoteBitrateEstimator,
+                        streamSSRC, /* mediaSSRC */ 0L, dest);
+    }
+
+    /**
+     * Makes an <tt>RTCPREMBPacket</tt> that provides receiver feedback to the
+     * endpoint from which we receive.
+     *
+     * @param remoteBitrateEstimator
+     * @param senderSSRC
+     * @param mediaSSRC
+     * @param dest
+     * @return an <tt>RTCPREMBPacket</tt> that provides receiver feedback to the
+     * endpoint from which we receive.
+     */
+    protected RTCPREMBPacket makeRTCPREMBPacket(
+            RemoteBitrateEstimator remoteBitrateEstimator,
+            long senderSSRC, long mediaSSRC, long[] dest)
+    {
+        // Exp & mantissa
+        long bitrate = remoteBitrateEstimator.getLatestEstimate();
+
+        if (bitrate == -1)
+        {
+            return null;
+        }
+        else
+        {
+            if (logger.isDebugEnabled())
+                logger.debug("Estimated bitrate: " + bitrate);
+
+            return new RTCPREMBPacket(senderSSRC, mediaSSRC, bitrate, dest);
+        }
     }
 
     /**
diff --git a/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MaxThroughputRTCPTerminationStrategy.java b/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MaxThroughputRTCPTerminationStrategy.java
index f838a439..e7aaad9c 100644
--- a/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MaxThroughputRTCPTerminationStrategy.java
+++ b/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MaxThroughputRTCPTerminationStrategy.java
@@ -16,6 +16,7 @@
 package org.jitsi.impl.neomedia.rtcp.termination.strategies;
 
 import org.jitsi.impl.neomedia.rtcp.*;
+import org.jitsi.service.neomedia.rtp.*;
 
 /**
  * Maximizes endpoint throughput. It does that by sending REMB messages with the
@@ -42,15 +43,15 @@ public class MaxThroughputRTCPTerminationStrategy
      * {@inheritDoc}
      */
     @Override
-    protected RTCPREMBPacket makeRTCPREMBPacket()
+    protected RTCPREMBPacket makeRTCPREMBPacket(
+            RemoteBitrateEstimator remoteBitrateEstimator,
+            long senderSSRC, long mediaSSRC, long[] dest)
     {
-        RTCPREMBPacket remb = super.makeRTCPREMBPacket();
-
-        if (remb != null)
-        {
-            remb.exp = MAX_EXP;
-            remb.mantissa = MAX_MANTISSA;
-        }
-        return remb;
+        return
+            new RTCPREMBPacket(
+                    senderSSRC,
+                    mediaSSRC,
+                    MAX_EXP, MAX_MANTISSA,
+                    dest);
     }
 }
diff --git a/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MinThroughputRTCPTerminationStrategy.java b/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MinThroughputRTCPTerminationStrategy.java
index e490f729..d1ad9852 100644
--- a/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MinThroughputRTCPTerminationStrategy.java
+++ b/src/org/jitsi/impl/neomedia/rtcp/termination/strategies/MinThroughputRTCPTerminationStrategy.java
@@ -16,6 +16,7 @@
 package org.jitsi.impl.neomedia.rtcp.termination.strategies;
 
 import org.jitsi.impl.neomedia.rtcp.*;
+import org.jitsi.service.neomedia.rtp.*;
 
 /**
  * Minimizes endpoint throughput. It does that by sending REMB messages with the
@@ -42,15 +43,15 @@ public class MinThroughputRTCPTerminationStrategy
      * {@inheritDoc}
      */
     @Override
-    protected RTCPREMBPacket makeRTCPREMBPacket()
+    protected RTCPREMBPacket makeRTCPREMBPacket(
+            RemoteBitrateEstimator remoteBitrateEstimator,
+            long senderSSRC, long mediaSSRC, long[] dest)
     {
-        RTCPREMBPacket remb = super.makeRTCPREMBPacket();
-
-        if (remb != null)
-        {
-            remb.exp = MIN_EXP;
-            remb.mantissa = MIN_MANTISSA;
-        }
-        return remb;
+        return
+            new RTCPREMBPacket(
+                    senderSSRC,
+                    mediaSSRC,
+                    MIN_EXP, MIN_MANTISSA,
+                    dest);
     }
 }
-- 
GitLab