Merge pull request #1576 from XevoInc/feature/issue-1569integration/stable_frame_rate

Implement SDL 0323: Align video streaming parameters with VideoStreamingCapability

3 files changed, 76 insertions, 49 deletions

diff --git a/android/sdl_android/src/main/java/com/smartdevicelink/encoder/VirtualDisplayEncoder.java b/android/sdl_android/src/main/java/com/smartdevicelink/encoder/VirtualDisplayEncoder.java
index a078531b2..7c8df5b98 100644
--- a/android/sdl_android/src/main/java/com/smartdevicelink/encoder/VirtualDisplayEncoder.java
+++ b/android/sdl_android/src/main/java/com/smartdevicelink/encoder/VirtualDisplayEncoder.java
@@ -125,14 +125,26 @@ public class VirtualDisplayEncoder {
         return this.streamingParams;
     }
 
+    /**
+     * This method is deprecated; setStreamingParams with having stableFrameRate should be used.
+     */
     @Deprecated
     public void setStreamingParams(int displayDensity, ImageResolution resolution, int frameRate, int bitrate, int interval, VideoStreamingFormat format) {
         this.streamingParams = new VideoStreamingParameters(displayDensity, frameRate, bitrate, interval, resolution, format);
     }
 
-    @SuppressWarnings("unused")
-    public void setStreamingParams(int displayDensity, ImageResolution resolution, int frameRate, int bitrate, int interval, VideoStreamingFormat format, boolean stableFrameRate) {
-        this.streamingParams = new VideoStreamingParameters(displayDensity, frameRate, bitrate, interval, resolution, format, stableFrameRate);
+    /**
+     * setter of every parameter in streamingParams.
+     * @param displayDensity
+     * @param resolution
+     * @param frameRate
+     * @param bitrate
+     * @param interval
+     * @param format
+     * @param stableFramerate
+     */
+    public void setStreamingParams(int displayDensity, ImageResolution resolution, int frameRate, int bitrate, int interval, VideoStreamingFormat format, boolean stableFramerate) {
+        this.streamingParams = new VideoStreamingParameters(displayDensity, frameRate, bitrate, interval, resolution, format, stableFramerate);
     }
 
     @SuppressWarnings("unused")
diff --git a/android/sdl_android/src/main/java/com/smartdevicelink/managers/video/VideoStreamManager.java b/android/sdl_android/src/main/java/com/smartdevicelink/managers/video/VideoStreamManager.java
index 714005041..a7ed506ab 100644
--- a/android/sdl_android/src/main/java/com/smartdevicelink/managers/video/VideoStreamManager.java
+++ b/android/sdl_android/src/main/java/com/smartdevicelink/managers/video/VideoStreamManager.java
@@ -294,38 +294,35 @@ public class VideoStreamManager extends BaseVideoStreamManager {
             stateMachine.transitionToState(StreamingStateMachine.ERROR);
             return;
         }
-        if (parameters == null) {
-            if (majorProtocolVersion >= 5) {
-                if (internalInterface.getSystemCapabilityManager() != null) {
-                    internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.VIDEO_STREAMING, new OnSystemCapabilityListener() {
-                        @Override
-                        public void onCapabilityRetrieved(Object capability) {
-                            VideoStreamingParameters params = new VideoStreamingParameters();
-                            params.update((VideoStreamingCapability) capability, vehicleMake);    //Streaming parameters are ready time to stream
-                            startStreaming(params, encrypted);
-                        }
+        // regardless of VideoStreamingParameters are specified or not, we should refer to VideoStreamingCapability.
+        if (majorProtocolVersion >= 5) {
+            if (internalInterface.getSystemCapabilityManager() != null) {
+                final VideoStreamingParameters params = ( parameters == null) ? new VideoStreamingParameters() : new VideoStreamingParameters(parameters);
+                internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.VIDEO_STREAMING, new OnSystemCapabilityListener() {
+                    @Override
+                    public void onCapabilityRetrieved(Object capability) {
+                        params.update((VideoStreamingCapability) capability, vehicleMake);    //Streaming parameters are ready time to stream
+                        startStreaming(params, encrypted);
+                    }
 
-                        @Override
-                        public void onError(String info) {
-                            stateMachine.transitionToState(StreamingStateMachine.ERROR);
-                            DebugTool.logError(TAG, "Error retrieving video streaming capability: " + info);
-                        }
-                    }, false);
-                }
-            } else {
-                //We just use default video streaming params
-                VideoStreamingParameters params = new VideoStreamingParameters();
-                DisplayCapabilities dispCap = null;
-                if (internalInterface.getSystemCapabilityManager() != null) {
-                    dispCap = (DisplayCapabilities) internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.DISPLAY, null, false);
-                }
-                if (dispCap != null) {
-                    params.setResolution(dispCap.getScreenParams().getImageResolution());
-                }
-                startStreaming(params, encrypted);
+                    @Override
+                    public void onError(String info) {
+                        stateMachine.transitionToState(StreamingStateMachine.ERROR);
+                        DebugTool.logError(TAG, "Error retrieving video streaming capability: " + info);
+                    }
+                }, false);
             }
         } else {
-            startStreaming(parameters, encrypted);
+            //We just use default video streaming params
+            VideoStreamingParameters params = (parameters == null) ? new VideoStreamingParameters() : new VideoStreamingParameters(parameters);
+            DisplayCapabilities dispCap = null;
+            if (internalInterface.getSystemCapabilityManager() != null) {
+                dispCap = (DisplayCapabilities) internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.DISPLAY, null, false);
+            }
+            if (dispCap != null) {
+                params.setResolution(dispCap.getScreenParams().getImageResolution());
+            }
+            startStreaming(params, encrypted);
         }
     }
 
diff --git a/base/src/main/java/com/smartdevicelink/streaming/video/VideoStreamingParameters.java b/base/src/main/java/com/smartdevicelink/streaming/video/VideoStreamingParameters.java
index 1d6e1a625..0424639c5 100644
--- a/base/src/main/java/com/smartdevicelink/streaming/video/VideoStreamingParameters.java
+++ b/base/src/main/java/com/smartdevicelink/streaming/video/VideoStreamingParameters.java
@@ -79,18 +79,28 @@ public class VideoStreamingParameters {
         format = new VideoStreamingFormat();
         format.setProtocol(DEFAULT_PROTOCOL);
         format.setCodec(DEFAULT_CODEC);
+        stableFrameRate = true;
     }
 
+    /**
+     * deprecated constructor of VideoStreamingParameters. This constructor will be removed in the future version.
+     * @param displayDensity
+     * @param frameRate
+     * @param bitrate
+     * @param interval
+     * @param resolution
+     * @param format
+     */
     @Deprecated
     public VideoStreamingParameters(int displayDensity, int frameRate, int bitrate, int interval,
                                     ImageResolution resolution, VideoStreamingFormat format) {
-        this.displayDensity = displayDensity;
-        this.frameRate = frameRate;
-        this.bitrate = bitrate;
-        this.interval = interval;
-        this.resolution = resolution;
-        this.format = format;
-        this.stableFrameRate = true;
+	    this.displayDensity = displayDensity;
+	    this.frameRate = frameRate;
+	    this.bitrate = bitrate;
+	    this.interval = interval;
+	    this.resolution = resolution;
+	    this.format = format;
+	    this.stableFrameRate = true;
     }
 
     /**
@@ -104,14 +114,14 @@ public class VideoStreamingParameters {
      * @param stableFrameRate
      */
     public VideoStreamingParameters(int displayDensity, int frameRate, int bitrate, int interval,
-                                    ImageResolution resolution, VideoStreamingFormat format, boolean stableFrameRate){
-	    this.displayDensity = displayDensity;
-	    this.frameRate = frameRate;
-	    this.bitrate = bitrate;
-	    this.interval = interval;
-	    this.resolution = resolution;
-	    this.format = format;
-	    this.stableFrameRate = stableFrameRate;
+                                    ImageResolution resolution, VideoStreamingFormat format, boolean stableFrameRate) {
+        this.displayDensity = displayDensity;
+        this.frameRate = frameRate;
+        this.bitrate = bitrate;
+        this.interval = interval;
+        this.resolution = resolution;
+        this.format = format;
+        this.stableFrameRate = stableFrameRate;
     }
 
     /**
@@ -171,9 +181,13 @@ public class VideoStreamingParameters {
      */
     public void update(VideoStreamingCapability capability, String vehicleMake) {
         if (capability.getMaxBitrate() != null) {
-            this.bitrate = capability.getMaxBitrate() * 1000;
+            // Taking lower value as per SDL 0323 :
+            // https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0323-align-VideoStreamingParameter-with-capability.md
+            int capableBitrateInKb = Math.min(Integer.MAX_VALUE / 1000, capability.getMaxBitrate());
+            this.bitrate = Math.min(this.bitrate, capableBitrateInKb * 1000);
         } // NOTE: the unit of maxBitrate in getSystemCapability is kbps.
         double scale = DEFAULT_SCALE;
+        // For resolution and scale, the capability values should be taken rather than parameters specified by developers.
         if (capability.getScale() != null) {
             scale = capability.getScale();
         }
@@ -198,12 +212,16 @@ public class VideoStreamingParameters {
             }
         }
         if (capability.getPreferredFPS() != null) {
-            this.frameRate = capability.getPreferredFPS();
+            // Taking lower value as per SDL 0323
+            this.frameRate = Math.min(this.frameRate, capability.getPreferredFPS());
         }
 
         // This should be the last call as it will return out once a suitable format is found
         final List<VideoStreamingFormat> formats = capability.getSupportedFormats();
         if (formats != null && formats.size() > 0) {
+            if (this.format != null && formats.contains(this.format)) {
+                return; // given format is supported, so no need to change.
+            }
             for (VideoStreamingFormat format : formats) {
                 for (VideoStreamingFormat currentlySupportedFormat : currentlySupportedFormats) {
                     if (currentlySupportedFormat.equals(format)) {