Fix issue with VPM and multi transports (#1022)

* Refactor VPM to specifically start video service

* tryVideoStream checks if service is started

* Fix unit test issue

* Update proxy VPM for manual service start flow

* Update javadoc for proxy non-deprecated video start

3 files changed, 85 insertions, 56 deletions

diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/video/VideoStreamManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/video/VideoStreamManagerTests.java
index e0febdb51..5e291c736 100644
--- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/video/VideoStreamManagerTests.java
+++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/video/VideoStreamManagerTests.java
@@ -40,6 +40,8 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 /**
@@ -116,7 +118,7 @@ public class VideoStreamManagerTests extends AndroidTestCase2 {
 	}
 
 	public void testHMILevelNotFull(){
-		ISdl internalInterface = mock(ISdl.class);
+		final ISdl internalInterface = mock(ISdl.class);
 
 		when(internalInterface.getProtocolVersion()).thenReturn((new Version(5,0,0)));
 		when(internalInterface.isCapabilitySupported(SystemCapabilityType.VIDEO_STREAMING)).thenReturn(true);
@@ -125,8 +127,10 @@ public class VideoStreamManagerTests extends AndroidTestCase2 {
 		videoStreamManager.start(new CompletionListener() {
 			@Override
 			public void onComplete(boolean success) {
-				assertNull(videoStreamManager.startVideoService(
-						new VideoStreamingParameters(), false));
+				VideoStreamingParameters params = new VideoStreamingParameters();
+				boolean encrypted = false;
+				videoStreamManager.startStreaming(params, encrypted);
+				verify(internalInterface, times(0)).startVideoService(params, encrypted);
 			}
 		});
 	}
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 7af1db458..4f8bac23c 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
@@ -66,7 +66,23 @@ public class VideoStreamManager extends BaseVideoStreamManager {
 		@Override
 		public void onServiceStarted(SdlSession session, SessionType type, boolean isEncrypted) {
 			if(SessionType.NAV.equals(type)){
-				stateMachine.transitionToState(StreamingStateMachine.READY);
+				if(session != null && session.getAcceptedVideoParams() != null){
+					parameters = session.getAcceptedVideoParams();
+				}
+				VideoStreamManager.this.streamListener = internalInterface.startVideoStream(isEncrypted, parameters);
+				if(streamListener == null){
+					Log.e(TAG, "Error starting video service");
+					stateMachine.transitionToState(StreamingStateMachine.ERROR);
+					return;
+				}
+				VideoStreamingCapability capability = (VideoStreamingCapability) internalInterface.getCapability(SystemCapabilityType.VIDEO_STREAMING);
+				if(capability != null && capability.getIsHapticSpatialDataSupported()){
+					hapticManager = new HapticInterfaceManager(internalInterface);
+				}
+				startEncoder();
+				stateMachine.transitionToState(StreamingStateMachine.STARTED);
+
+
 			}
 		}
 
@@ -229,31 +245,6 @@ public class VideoStreamManager extends BaseVideoStreamManager {
 		}
 	}
 
-	/**
-	 * Opens a video service (service type 11) and subsequently provides an IVideoStreamListener
-	 * to the app to send video data. The supplied VideoStreamingParameters will be set as desired paramaters
-	 * that will be used to negotiate
-	 *
-	 * @param parameters  Video streaming parameters including: codec which will be used for streaming (currently, only
-	 *                    VideoStreamingCodec.H264 is accepted), height and width of the video in pixels.
-	 * @param encrypted Specify true if packets on this service have to be encrypted
-	 *
-	 * @return IVideoStreamListener interface if service is opened successfully and streaming is
-	 *         started, null otherwise
-	 */
-	protected IVideoStreamListener startVideoService(VideoStreamingParameters parameters, boolean encrypted){
-		if(hmiLevel != HMILevel.HMI_FULL){
-			Log.e(TAG, "Cannot start video service if HMILevel is not FULL.");
-			return null;
-		}
-		IVideoStreamListener listener = internalInterface.startVideoStream(encrypted, parameters);
-		if(listener != null){
-			stateMachine.transitionToState(StreamingStateMachine.STARTED);
-		}else{
-			stateMachine.transitionToState(StreamingStateMachine.ERROR);
-		}
-		return listener;
-	}
 
 	/**
 	 * Starts video service, sets up encoder, haptic manager, and remote display. Begins streaming the remote display.
@@ -261,19 +252,16 @@ public class VideoStreamManager extends BaseVideoStreamManager {
 	 *                    VideoStreamingCodec.H264 is accepted), height and width of the video in pixels.
 	 * @param encrypted Specify true if packets on this service have to be encrypted
 	 */
-	private void startStreaming(VideoStreamingParameters parameters, boolean encrypted){
+	protected void startStreaming(VideoStreamingParameters parameters, boolean encrypted){
 		this.parameters = parameters;
-		this.streamListener = startVideoService(parameters, encrypted);
-		if(streamListener == null){
-			Log.e(TAG, "Error starting video service");
-			stateMachine.transitionToState(StreamingStateMachine.ERROR);
+		if(hmiLevel != HMILevel.HMI_FULL){
+			Log.e(TAG, "Cannot start video service if HMILevel is not FULL.");
 			return;
 		}
-		VideoStreamingCapability capability = (VideoStreamingCapability) internalInterface.getCapability(SystemCapabilityType.VIDEO_STREAMING);
-		if(capability != null && capability.getIsHapticSpatialDataSupported()){
-			hapticManager = new HapticInterfaceManager(internalInterface);
-		}
-		startEncoder();
+		//Start the video service
+		this.internalInterface.startVideoService(parameters, encrypted);
+
+
 	}
 
 	/**
diff --git a/android/sdl_android/src/main/java/com/smartdevicelink/proxy/SdlProxyBase.java b/android/sdl_android/src/main/java/com/smartdevicelink/proxy/SdlProxyBase.java
index acf697a9f..e622ae0dd 100644
--- a/android/sdl_android/src/main/java/com/smartdevicelink/proxy/SdlProxyBase.java
+++ b/android/sdl_android/src/main/java/com/smartdevicelink/proxy/SdlProxyBase.java
@@ -4895,8 +4895,12 @@ public abstract class SdlProxyBase<proxyListenerType extends IProxyListenerBase>
 

     /**

      * Opens a video service (service type 11) and subsequently provides an IVideoStreamListener

-     * to the app to send video data. The supplied VideoStreamingParameters will be set as desired paramaters

+     * to the app to send video data. The supplied VideoStreamingParameters will be set as desired parameters

 	 * that will be used to negotiate

+	 *

+	 * <br><br><b>NOTE: IF USING SECONDARY TRANSPORTS, THE VIDEO SERVICE MUST BE STARTED BEFORE CALLING THIS

+	 * THIS METHOD. USE A `ISdlServiceListener` TO BE NOTIFIED THAT IT STARTS THEN CALL THIS METHOD TO

+	 * START STREAMING. ADD A LISTENER USE {@link #addServiceListener(SessionType, ISdlServiceListener)}.</b>

      *

      * @param isEncrypted Specify true if packets on this service have to be encrypted

      * @param parameters  Video streaming parameters including: codec which will be used for streaming (currently, only

@@ -4904,6 +4908,8 @@ public abstract class SdlProxyBase<proxyListenerType extends IProxyListenerBase>
      *

      * @return IVideoStreamListener interface if service is opened successfully and streaming is

      *         started, null otherwise

+	 *

+	 * @see ISdlServiceListener

      */

     @SuppressWarnings("unused")

     public IVideoStreamListener startVideoStream(boolean isEncrypted, VideoStreamingParameters parameters) {

@@ -5094,21 +5100,24 @@ public abstract class SdlProxyBase<proxyListenerType extends IProxyListenerBase>
             return null;

         }

 

-		sdlSession.setDesiredVideoParams(parameters);

+		if(!navServiceStartResponseReceived || !navServiceStartResponse //If we haven't started the service before

+				|| (navServiceStartResponse && isEncrypted && !sdlSession.isServiceProtected(SessionType.NAV))) { //Or the service has been started but we'd like to start an encrypted one

+			sdlSession.setDesiredVideoParams(parameters);

 

-		navServiceStartResponseReceived = false;

-		navServiceStartResponse = false;

-		navServiceStartRejectedParams = null;

+			navServiceStartResponseReceived = false;

+			navServiceStartResponse = false;

+			navServiceStartRejectedParams = null;

 

-		sdlSession.startService(SessionType.NAV, sdlSession.getSessionId(), isEncrypted);

+			sdlSession.startService(SessionType.NAV, sdlSession.getSessionId(), isEncrypted);

 

-		FutureTask<Void> fTask = createFutureTask(new CallableMethod(RESPONSE_WAIT_TIME));

-		ScheduledExecutorService scheduler = createScheduler();

-		scheduler.execute(fTask);

+			FutureTask<Void> fTask = createFutureTask(new CallableMethod(RESPONSE_WAIT_TIME));

+			ScheduledExecutorService scheduler = createScheduler();

+			scheduler.execute(fTask);

 

-		//noinspection StatementWithEmptyBody

-        while (!navServiceStartResponseReceived && !fTask.isDone());

-        scheduler.shutdown();

+			//noinspection StatementWithEmptyBody

+			while (!navServiceStartResponseReceived && !fTask.isDone()) ;

+			scheduler.shutdown();

+		}

 

         if (navServiceStartResponse) {

 			if(protocolVersion!= null && protocolVersion.getMajor() < 5){ //Versions 1-4 do not support streaming parameter negotiations

@@ -7827,6 +7836,7 @@ public abstract class SdlProxyBase<proxyListenerType extends IProxyListenerBase>
 		float[] touchScalar = {1.0f,1.0f}; //x, y

 		private HapticInterfaceManager hapticManager;

 		SdlMotionEvent sdlMotionEvent = null;

+		VideoStreamingParameters videoStreamingParameters;

 

 		public VideoStreamingManager(Context context,ISdl iSdl){

 			this.context = context;

@@ -7847,8 +7857,30 @@ public abstract class SdlProxyBase<proxyListenerType extends IProxyListenerBase>
 			});

 		}

 

+		/**

+		 * Starts the video service, caches the supplied params and prepares for the stream to start.

+		 * @param remoteDisplayClass the extension of SdlRemoteDisplay that will be streamed

+		 * @param parameters desired video streaming params

+		 * @param encrypted if the service is to be encrypted or not

+		 */

 		public void startVideoStreaming(Class<? extends SdlRemoteDisplay> remoteDisplayClass, VideoStreamingParameters parameters, boolean encrypted){

-			streamListener = startVideoStream(encrypted,parameters);

+			this.remoteDisplayClass = remoteDisplayClass;

+			this.videoStreamingParameters = parameters;

+			//Make sure the service is started, allows multi transports to connect and register without timing out

+			internalInterface.startVideoService(parameters, encrypted);

+			//After this, look to the

+		}

+

+		/**

+		 * The video service should already be started at this point. Once called, it will start

+		 * the encoders and fire up the remote display supplied by the user

+		 * @param parameters

+		 * @param encrypted

+		 */

+		private void startStream(VideoStreamingParameters parameters, boolean encrypted){

+			//Start the service first

+			//streamListener = startVideoStream(encrypted,parameters);d

+			streamListener = sdlSession.startVideoStream();

 			if(streamListener == null){

 				Log.e(TAG, "Error starting video service");

 				return;

@@ -7857,9 +7889,9 @@ public abstract class SdlProxyBase<proxyListenerType extends IProxyListenerBase>
 			if(capability != null && capability.getIsHapticSpatialDataSupported()){

 				hapticManager = new HapticInterfaceManager(internalInterface);

 			}

-			this.remoteDisplayClass = remoteDisplayClass;

+

 			try {

-				encoder.init(context,streamListener,parameters);

+				encoder.init(context, streamListener, parameters);

 				//We are all set so we can start streaming at at this point

 				encoder.start();

 				//Encoder should be up and running

@@ -7962,8 +7994,13 @@ public abstract class SdlProxyBase<proxyListenerType extends IProxyListenerBase>
 

 		@Override

 		public void onServiceStarted(SdlSession session, SessionType type, boolean isEncrypted) {

-

-

+			if(SessionType.NAV.equals(type) && session != null ){

+				DebugTool.logInfo("Video service has been started. Starting video stream from proxy");

+				if(session.getAcceptedVideoParams() != null){

+					videoStreamingParameters = session.getAcceptedVideoParams();

+				}

+				startStream(videoStreamingParameters, isEncrypted);

+			}

 		}

 

 		@Override