Sdl_evolution: [Accepted with Revisions] SDL 0296 - Possibility to update video streaming capabilities during ignition cycle

[theresalech]

Hello SDL community,

The review of the revised proposal "SDL 0296 - Possibility to update video streaming capabilities during ignition cycle" begins now and runs through June 30, 2020. The original review of this proposal took place March 24 - June 16, 2020. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0296-Update-video-streaming-capabilities-during-ignition-cycle.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

https://github.com/smartdevicelink/sdl_evolution/issues/983

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

• Is the problem being addressed significant enough to warrant a change to SDL?
• Does this proposal fit well with the feel and direction of SDL?
• If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
  Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
[email protected]

[JackLivio]

1.

Currently, REMOTE_CONTROL and APP_SERVICES capabilities can already be updated through it, so the idea is to extend this list by VIDEO_STREAMING capabilities.

APP_SERVICES capabilities are not updated through the OnSystemCapabilityUpdated notification from HMI. DISPLAYS and REMOTE_CONTROL are updated through this rpc.

2.

I don't agree it is necessary to add the OnSystemCapabilitySubscription. The HMI should always send capability updates when they change, there is no need to notify the HMI it should start sending notifications.

[joeljfischer]

3.

In "Mobile libraries changes (Java Suite and iOS)", you state:

Mobile applications should be able to update the streaming content window to the new video streaming capabilities received in OnSystemCapabilityUpdated.

It's not just the mobile application that needs to update. Changes will need to occur in the video lifecycle managers to support updating values and passing those down to the app. Additional public APIs may be needed to support this case (telling the developer to update their view based on new streaming values). Please investigate this and update the proposal accordingly.

[dboltovskyi]

@JackLivio

1. Thank you for catching this.
   Proposal will be updated: APP_SERVICES item - excluded, DISPLAYS - included.

   1. 
      Dynamic resolution switching requires some actions in HMI and Mobile application both.
      The reason to have OnSystemCapabilitySubscription notification is to let HMI know if a particular application supports resolution switching.
      Hence HMI will be able to send OnSystemCapabilityUpdated notification with updated VIDEO_STREAMING capabilities to specific application only.
      In this way both HMI and Mobile application will be in sync regarding current resolution.

[Sohei-Suzuki-Nexty]

4.What are the use cases for dynamically updating the values of these parameters during the ignition cycle?

[atiwari9]

@Sohei-Suzuki-Nexty

1. Some headunits/HMI has option to enter some sort of Fullscreen or collapsible projection mode upon user's input. This proposal attempts to address those scenarios.

[JackLivio]

@dboltovskyi

1. Older Apps that won't support this feature can already send a GetSystemCapabaility request with subscribe:true. I don't think that subscribing to the video capability means that the app can support resolution switching.

[dboltovskyi]

@JackLivio

1. Another approach is to add some new parameter to RegisterAppInterface request, e.g. isResolutionSwitchingSupported which will be transferred to HMI within BC.OnAppRegistered notification (as part of HMIApplication struct).
   In this way OnSystemCapabilitySubscription notification is not required

[theresalech]

The Steering Committee acknowledged that items 1 and 2 in the comments have been agreed upon and will be necessary revisions. However, as items 3 and 4 are still awaiting resolution/confirmation, the Steering Committee voted to keep this proposal in review for another week. The author will respond to item 3, and the commenter will confirm if their questions in item 4 have been answered.

[Sohei-Suzuki-Nexty]

@atiwari9 -san

1. Is "collapsible projection mode" a function that shrinks the screen?
   In that case, are you assuming a use case where the app side supports various scales of display according to the HU screen size change?

[atiwari9]

@Sohei-Suzuki-Nexty

1. I just mentioned an example of why HMI would need to change the resolution for projection window. And yes for this use case to be viable, app would need to support changes in any parameters in VideoStreamingCapability struct .

[joeygrover]

2. I'm sorry to come in late, but how is this flag going to be populated? Does the developer set this if they support it? Where do they do that? Does the library automatically set this flag? How does this affect the HMIs behavior? If an app connects that doesn't support the feature how will it handle that as compared to one that does? I understand the new values won't be sent, but what is the HMI supposed to do in place of the feature? This seems like another arbitrary flag that adds complications and extra bookkeeping and therefore shouldn't be added.

3. Agreeing with Joel here. There is a lot of hand waving taking place here. Without including how the mobile libraries will implement this feature I do not feel the proposal should be accepted. Leaving this as a future detail simply limits the feature from ever being used until those modifications are proposed.

5. I have a feeling larger changes are going to have to be implemented to support this, including protocol changes. Based on the current protocol spec the actual resolution is determined between the StartService and StartServiceACK for the video service. The provided information in the VIDEO_STREAMING_CAPABILITY gives the app information on what to use as baseline values to start the video service. Changing these values mid stream will result in additional needs for altering the stream that are not provided in this proposal.

[kostyaBoss]

@joeljfischer
3.

Changes will need to occur in the video lifecycle managers to support updating values and passing those down to the app

We with @dboltovskyi spent some time developing and testing POC for the scenario when VideoStreamingCapabilities could be changed before streaming. During development, no additional lifecycle changes were provided, though there was introduced another listener and subscription to value in order to handle incoming specific OnSystemCapabilityUpdated notification and pass new data to VideoStreamManager. I believe, code impact related to updating of streaming capabilities in the middle of the stream should be minor, but POC for this kind of case is still in progress

Additional public APIs may be needed to support this case (telling the developer to update their view based on new streaming values). Please investigate this and update the proposal accordingly.

In terms of this proposal, no public API available for mobile developers should be changed. We expect the changes only in the Audio/Video stream managers, though the lifecycle is not supposed to be changed

[Sohei-Suzuki-Nexty]

@atiwari9 -san

1. If you simply want to minimize the display, isn't it better to perform the process of minimizing on the HU side? Also, if the app matches the display according to the screen of the HU, does this mean that the app needs to prepare various designs? For example, if you want to display on a half screen horizontally, don’t you need to consider a completely different design from a full screen? Not exactly the same, but I think it will be a form that supports both portrait and landscape.
   However, this may increase the burden on app developers.

[atiwari9]

@Sohei-Suzuki-Nexty

1. Idea of this proposal is to include the capability that is scalable enough. App developers and OEM's would of course need to optimize the integration part. If your concerns are about switching between Landscape and Portrait orientation, i believe most projection apps adapt to these layouts for different phone orientation and form factors etc. Still, if an app does nto support it, it has the option to communicate that to HMI as discussed in point 2 in this thread.

[dboltovskyi]

@joeygrover

1. By default proposed isResolutionSwitchingSupported flag is going to be set to true automatically by library. However app developer would still be able to override default value. In case if this flag is set to false HMI may not allow collapsing of the app's window.

2. We're going to provide more details in revision of proposal upon updates in mobile library once PoC is ready.
   Some results have already been mentioned in https://github.com/smartdevicelink/sdl_evolution/issues/983#issuecomment-610072231

5.
Having of StartServiceACK response with updated values - it's another approach which could be used instead of OnSystemCapabilityUpdated notification. We're going to mention it as alternative in revision of proposal.
Existing height and width response parameters may need to be extended by diagonalScreenSize, pixelPerInch and scale ones.
Also some analysis required to check if this solution covers the case of changing capabilities in the middle of streaming process.

[joeygrover]

2. I would like to see this section still flushed out more. While I know each OEM can and will have different UX requirements, it is very helpful to all OEMs where there is at least a reference that can be used. For this piece, I would really like to see what the HMI will do between these different cases. Mobile dev conners can be moved to 3.

3. Ok, yes I would like to see how this is handled and will reserve additional comments until this is included.

5. I don't think I explained the situation well enough. The solution in the proposal is ok, but there are protocol level changes that need to happen too. The actual resolution of the stream is negotiated in the StartService and StartServiceAck packets for the video service. So if the resolution changed, the HMI sends the OnSystemCapabilityUpdated with the new resolution, the library will have to inform the VideoStreamingManager, it will have to send a new StartService with the potential new resolution supplied in the OnSystemCapabilityUpdated, the IVI system will hav etc respond with a StartServiceAck that confirms this new resolution of the stream. Then the video streaming manager will have to update the supplied display/codecs/etc. Therefore this is not an alternative, but an extension that should be required as well.

[Shohei-Kawano]

@atiwari9 -san

1. I understand your explanation. Thank you.
   We are concerned that various display patterns depending on OEMs may increase the testing of App developers.
   It is expected that the above will occur if OEMs are able to set up freely. So I felt that it was necessary to prepare a pattern for the parameters.
   Doing so may reduce the testing because App developers only need to consider the pattern.

[atiwari9]

@Shohei-Kawano

1. I disagree, by this logic app developers would need to support infinite videoStreamingCapabilities combinations even without this proposal as well an OEM can pick any of the resolution. But well that's not the case isn't it. In reality OEM/App developer perform the integration testing and ensure that all the use cases are working fine. That'd be the case with dynamically switching videoStreamingCapabilities as well, most likely an OEM would have two different resolutions to switch between. But we can't limit what videoStreamingCapabilities an OEM can or can't use. It'd be up to OEM and app developers to do the integration testing for supported videoStreamingCapabilities, which of course is a finite number.

[joeljfischer]

4. @atiwari9 I don't think that your logic works out. Even if app developers did have to support "infinite" capability combinations (though in reality they're mostly various versions of 16:9 or close to it), now you're adding exponentially onto the infinite amount. Stating that it's up to the app developer to do their testing for a given OEM ignores the fact that if you are adding "smaller" windows, the aspect ratios of those windows are going to be all over the place. Some may be portrait, some may be square, some may be landscape but much smaller.

The fact that you're greatly adding onto the burden of app developers is the real issue with this point. App developers go from having their aspect ratio be "landscape, around 16:9," to having their views required to be able to stream in many additional aspect ratios and screen sizes (1 inch square, 3 inch high 1 inch wide, etc.).

I don't think that hand-waving away that fact by "app developers will have to do integration testing" is enough to account for the large increase in the burden on developers you're adding with this point. It _at least_ needs to be addressed in the downsides that you're adding an exponential burden onto navigation app developers.

[theresalech]

A quorum was not present during the 2020-04-07 Steering Committee Meeting, so this proposal will remain in review until our meeting on 2020-04-14.

[atiwari9]

@joeljfischer

1. Ok, so what is the argument here, that App Developers might need to support both portrait and landscape modes? Or is it that the window size would be smaller etc? This proposal just aims to add capability for HMI to be able to switch from one resolution to the other upon some trigger(which HMI decides). Now for a practical use case, it would be:

   1. Fixed Regular(smaller) screen to Fixed Full screen and vice versa
   2. Fixed Regular(larger) screen to Fixed preview(smaller) screen and vice versa

And of course HMI could have Regular screen layout in either Portrait or Landscape mode. We do not tell HMI that it can only have a landscape or portrait screen. In reality it is up to HMI to decide what layout is best. So in this case, if OEM1 wants portrait and OEM2 wants landscape, app partner would still need to support both? More over since apps primarily run on phone, which support split screen/PIP/Portrait/Landscape modes all in one phone, i do not see that doing a basic use case of switching resolution is that hard for app to adapt to.

Only valid point seems to be super small resolutions which apps might not be able to adapt to due to variety of content an interactions etc(if app does not support PIP or floating window on phone anyways). In that case you may propose to increase minValue for resolution params to certain plausible value which we can discuss.

Refer this video for Split screen on Android

[Sohei-Suzuki-Nexty]

@atiwari9 -san

1. We think that it is better to handle the process on the HU side if you simply want to display a minimized/smaller screen. We would to know what are your thoughts on this.

[atiwari9]

@Sohei-Suzuki-Nexty

1. Can you please elaborate on this with some example and actions flow diagram?

[joeygrover]

4. @atiwari9 I believe he is simply saying since the playback window intends to be smaller, why not just have the IVI system take the higher resolution stream from the application and convert it for the smaller window resolution.

5. Also still looking for a response here.

[dboltovskyi]

@joeljfischer @joeygrover @Sohei-Suzuki-Nexty

4.
Please let me put more details here.
Let's imagine we have projection mobile application which streams the data to IVI system.
By some action on IVI system this application is switched from Full to Minimized mode.

We may have the following examples:

In example 1) the whole video content is resized.
Video frame as well as UI elements (buttons and labels) are resized proportionally by the same scale (e.g. 2)
Currently this can be implemented completely in IVI system.

In example 2) video frame is resized, however size (in inches) of UI elements remains pretty much the same as in Full screen mode.
Retaining the size of UI elements was the main goal of 0179-pixel-density-and-scale proposal.
However this can not be achieved without updates on Mobile side.

[kostyaBoss]

1. What we've done in Java library related to Android
   In VideoStreamManager was modified constructor, and additional subscription, like this :

this.internalInterface.addOnSystemCapabilityListener(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
                VideoStreamManager.this.parameters = params;
                virtualDisplayEncoder.setNewParams(params); // here setting new params
                if (isStreaming()) {
                    stopStreaming();
                }
                resumeStreaming();
            }

            @Override
            public void onError(String info) {

            }
        });

So that we are subscribed to ON_SYSTEM_CAPABILITY_UPDATED,
and what goes in stopStreaming() and resumeStreaming() is :

for stopStreaming()

if(remoteDisplay!=null){
    remoteDisplay.stop();
}
if(virtualDisplayEncoder!=null){
    virtualDisplayEncoder.shutDown();
}
stateMachine.transitionToState(StreamingStateMachine.STOPPED);

and for resumeStreaming()

if(stateMachine.getState() != StreamingStateMachine.STOPPED){
    return;
}
startEncoder();

where startEncoder() restarts SdlRemoteDisplay

the only problem here is in restarting, while this process SdlRemoteDisplay is recreated, thus state of View would be lost. Right now we are working on handling of this scenario, trying to implement new Surface creation with new Parameters for SdlRemoteDisplay and just resizing SdlRemoteDisplay

[dboltovskyi]

@joeygrover

1. 
   Currently we do not expect the changes on a protocol level.
   We also do not expect new StartService/ StartServiceAck messages from Mobile and SDL.
   Please take a look at https://github.com/smartdevicelink/sdl_evolution/issues/983#issuecomment-613582128

We suppose currently when mobile library receives StartServiceAck from SDL it performs some validation of parameters.
The same validation logic may be applied when mobile library receives OnSystemCapabilityUpdated with new video capabilities.

[theresalech]

The Steering Committee voted to keep this proposal in review, so SDLC Members and the Project Maintainer can respond to the latest comments from the author.

[theresalech]

The PM worked to summarize our feedback to the open items below. Thanks!
2.
When you show this flow in the revised proposal, please make sure default for isResolutionSwitchingSupported is FALSE.

3.
This solution doesn't allow the developer to make changes to their UI size. Again, for the solution this proposal is trying to reach, Public API changes will be needed. The current code solution would actually result in the same outcome as the IVI simply resizing the stream.

4.
The options presented display a poor UX for option 2 that should be clear. Seeing a navigation app being pushed to a minimized view with having its buttons overtake most of the display area would be a very awful UX. The user wouldn't be able to see the map, only buttons. Whereas, simply resizing the video stream is a much less complicated solution and makes using a navigation app in smaller window more viable. Instead of recreating these UI elements to fit in an infinite set of resolutions, it makes more sense to resize the given stream, and when a user presses on the window, bring the app into HMI_FULL and display the app in the normal resolution.

5.
If the solution is accepted with changing resolutions on the mobile side, protocol changes are needed. The stream params must be renegotiated. The resolution can't simply be changed without notification. Please update the solution to reflect this requirement.

[theresalech]

Per the author's request, the Steering Committee voted to defer this proposal. This will allow the author time to investigate a new solution that resolves the issues identified by the Project Maintainer in this comment. Once the author has identified a new solution, we can bring this proposal back in review to discuss their proposed revisions for which the proposal can be returned.

[theresalech]

The author has advised that they have investigated alternative solutions and are ready to bring this proposal back in to review to discuss those solutions/their proposed revisions. This proposal will now be in review until 2020-05-12.

[dboltovskyi]

We would appreciate if Steering Committee be able to review this comment and provide some feedback.

We would like to stress that the main goal of the proposal is to cover the following use cases related to dynamic resolution switching:

• Full screen view
• Picture-in-Picture
• Preview
• Split screen
• Collapsed view

The picture shown previously in case 2. (big buttons) is just an example for scale=2.
In case if scale=1.25 (or 1.5) UI elements won't look so large and they will be more usable comparing to scale=1.

HMI may follow the behavior of case 1. (small buttons) if mobile application does not support dynamic resolution switching.

In order to support case 2. we propose the following new solution.

Part 1) Negotiation of video streaming capabilities (VSCs) between HMI and Mobile application

a) Introduce a new optional availableVideoStreamingCapabilities array parameter under SystemCapability structure.
It would be populated by HMI within UI.GetCapabilities response and includes all available video streaming capabilities that HMI may handle.

b) Mobile application would be able to obtain these available VSCs through GetSystemCapability request/response of VIDEO_STREAMING type. Within this RPC mobile application also be able to subscribe to VSC updates.

c) Introduce new SelectDisplayCapability RPC

+ <function name="SelectDisplayCapability" functionID="SelectDisplayCapabilityID" messagetype="request" since="6.x">
+     <description>Provides an information about supported display capabilities</description>
+     <param name="supportedVideoStreamingCapabilities" type="VideoStreamingCapability" array="true" minsize="1" maxsize="100" mandatory="true">
+         <description>List of supported video streaming capabilities</description>
+     </param>
+ </function>

Through this RPC Mobile application would be able to provide list of supported VSCs to HMI. And HMI would adapt it's internal behavior accordingly.

Part 2) Resolution Switching

In order to notify Mobile application about current VSC HMI may use existing OnSystemCapabilityUpdated notification of VIDEO_STREAMING type with existing videoStreamingCapability parameter populated.

Mobile application would receive such updates if it has been previously subscribed to.

Resolution switching would be done according to existing stop streaming and start streaming flows:

[theresalech]

Hi @dboltovskyi, thank you for providing this new solution. The Project Maintainer has reviewed and provided feedback below:

3. The proposal still needs to address the original concern about developers changing the UI size and Public API changes needed (including manager changes).

6. SelectDisplayCapability RPC should be renamed because it's not a Display Capability and you cannot select it. This new RPC should also be reflected in Resolution Switching diagram

7. We need to handle the case for apps that won't support this feature. Can you please provide information on how this would work?

8. We should not add two parameters under the SystemCapability structure for the same capability type. This will cause large issues in the library manager layer. Either this should be added as a nested array in the VideoStreamingCapabilities struct itself or a new SystemCapabilityType be defined.

9. We would like the proposal to include that the following should be added to the HMI Integration Guidelines:

1. If the change is a PIP-type change, scale the current video stream to the size you desire without going through this flow. Touches should never be passed to the app. Either a system menu/buttons should be displayed when selected, or the selection should bring the user immediately back to the full-screen app.
2. If the change is a split-screen type change (cannot be handled by scaling the original video), then go through this flow. If the app does not support your custom split-resolution, either don't allow the user to put the app in split screen, or scale the video stream to a size that fits the window and use "black bars" to fill the rest of the window. Touches should never be passed to the app. Either a system menu / buttons should be displayed when selected, or the selection should bring the user immediately back to the full-screen app.

[dboltovskyi]

@theresalech Thank you for the review.

1. 
   We have completely working PoC for Android and details upon changes in mobile library going to be provided in separate comment.

6.
Agree, current SelectDisplayCapability is not the best name. We have to find out more suitable one (e.g.: SupportedCapabilities)
I would propose to show VSCs negotiation on a separate diagram since Negotiation and Resolution switching sequences are not strictly depends on each other.

7.
In case if app doesn't support this feature it won't send SelectDisplayCapability request with supported VSCs.
HMI may handle this app in 2 ways:

• Disable any kind of resolution switching (PIP, Split etc.) Only Full screen view would be available. This is current behavior which doesn't require any changes on HMI side.
• Resize app's window as described in https://github.com/smartdevicelink/sdl_evolution/issues/983#issuecomment-613576877 in 1st example. This would require an implementation on HMI side.

8.
New array parameter describes all available VSCs supported by HMI, however existing old non-array parameter is related to current VSC selected by HMI.

9.
Scaling (increasing) of UI elements for a smaller app's window is the main idea of this proposal.
It's assumed touchable UI elements (buttons) would work in this case as well.
Available and supported VSCs would be aligned during negotiation procedure.
And it's up to HMI is to support (or not) touch events for a particular resolution.

[kostyaBoss]

@theresalech 3. Attaching most important code changes from POC with detailed description of why this change is needed and how it works

In SDLManager class :

@Override
public void stopVideoService(boolean withPendingRestart) {
    if(proxy.getIsConnected()){
        proxy.endVideoStream();
    }
    if (withPendingRestart && videoStreamManager != null && proxy.getIsConnected()){
        proxy.startVideoService(videoStreamManager.getLastCachedIsEncrypted(),
                videoStreamManager.getLastCachedStreamingParameters());
    }
}

As you can see, here we’ve extended startVideoService with VideoStreamingParams to give actual values for streaming and inrtuced new boolean in stopVideoService - so that entity could know if pending restart is planned

In SDLRemoteDisplay class :

public void resizeView(final int newWidth, final int newHeight) {
  uiHandler.post(new Runnable() {
    @Override
    public void run() {
      try {
        Constructor<? extends ViewGroup.LayoutParams> ctor =
            mainView.getLayoutParams().getClass().getDeclaredConstructor(int.class, int.class);
        mainView.setLayoutParams(ctor.newInstance(newWidth, newHeight));
        mainView.requestLayout();
        invalidate();
      } catch (Exception e) {
        e.printStackTrace();
      }
    }
  });
}

SDLRemoteDisplay (actually one for extension by third-party developers) is extended with resize method in order to provide resizing only layout, keeping all UI controls (Buttons, TextViews) with same size. In this way goal of our type of scaling is achieved

In VirtualDisplayEncoder class :

public void pauseEncoding(){
 try {
      if (encoderThread != null) {
        encoderThread.interrupt();
        encoderThread = null;
      }
      if (mVideoEncoder != null) {
        mVideoEncoder.stop();
        mVideoEncoder.release();
        mVideoEncoder = null;
      }
      if (inputSurface != null) {
        inputSurface.release();
        inputSurface = null;
      }
}

This method is quite similar to existing stop method, though the huge difference - this one is not releasing VirtualDisplay. If we ever do that, according to Android documentation, everything else will be destroyed (surface, remoteDisplay), so that it will produce loss of state, because RemoteDisplay once disposed should be reinstantiated

Another change in VirtualDisplayEncoder

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && virtualDisplay != null) {
  virtualDisplay.setSurface(null);
  virtualDisplay.setSurface(inputSurface);
}
else {
  virtualDisplay = mDisplayManager.createVirtualDisplay(TAG,
      predefined_width, predefined_height,
      streamingParams.getDisplayDensity(), inputSurface, DisplayManager.VIRTUAL_DISPLAY_FLAG_PRESENTATION);
}

In encoder we are checking if current VirtualDisplay is not null. If it is - we should create one, otherwise - reattach canvas which was released by pauseEncoding

And in VideoStreamingManager :

this.internalInterface.addOnSystemCapabilityListener(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
        VideoStreamManager.this.parameters = params;
        virtualDisplayEncoder.setStreamingParams(params);
        restartStreaming();
    }
    @Override
    public void onError(String info) {
    }
});
public void restartStreaming(){
    if (this.isStreaming()) {
        if(virtualDisplayEncoder!=null){
            virtualDisplayEncoder.pauseEncoding();
        }
        stateMachine.transitionToState(StreamingStateMachine.STOPPED);
        this.internalInterface.stopVideoService(true);
    }
}

In VideoStreamingManager we have subscription to VideoStreaming capability in order to trigger restart process using provided flow. Also restartStreaming disposes current encoder, surface and interrupts encoder thread to stop frame sending. After - sends signal to internalInterface passing withPendingRestart = true value in order to restart service

[theresalech]

Hi @dboltovskyi and @kostyaBoss, please find responses from the PM below:

3. Thank you. Can you please provide the changes required for iOS? We will need to be able to review both implementations simultaneously to ensure alignment among the libraries.

6. We would like to recommend a generalized way for the mobile device to notify Core about its capabilities similar to how Core notifies an app about its System Capabilities. When an app wants to notify Core about its capabilities, the app should send Core a notification, OnAppCapabilitiesUpdated . This would be used in place of proposed rpc SelectDisplayCapability. Note it is assumed that Core is automatically subscribed to this notification.

    <function name="OnAppCapabilityUpdated" functionID="OnAppCapabilityUpdatedID" messagetype="notification" since="x.x">
        <description>A notification to inform SDL Core that a specific app capability has changed.</description>
        <param name="appCapability" type="AppCapability" mandatory="true">
            <description>The app capability that has been updated</description>
        </param>
    </function>

    <struct name="AppCapability" since="x.x">
        <param name="capabilityType" type="AppCapabilityType" mandatory="true">
            <description>Used as a descriptor of what data to expect in this struct. The corresponding param to this enum should be included and the only other param included.</description>
        </param>
        <param name="videoStreamingCapability" type="AppVideoStreamingCapability" mandatory="false">
            <description>Describes supported capabilities for video streaming </description>
        </param>
    </struct>

   <enum name="AppCapabilityType" since="x.x">
        <description>Enumerations of all available app capability types</description>
        <element name="VIDEO_STREAMING"/>
    </enum>

    <struct name="AppVideoStreamingCapability" since="x.x">
        <description>Contains information about this app's video streaming capabilities.</description>
        <param name="supportedResolutions" type="ImageResolution" mandatory="false" array="true">
            <description>Supported streaming resolutions</description>
        </param>
    </struct>

7. Sounds good, thanks.

8. We understand what the proposal is stating and how you see these parameters working. However, we see issues with this implementation, as noted in our comment previously. Please advise how you can address these issues (either this should be added as a nested array in the VideoStreamingCapabilities struct itself or a new SystemCapabilityType be defined.).

9. We were under the impression that the purpose of this proposal was to resize the video streaming window, not to scale the UI elements. If the purpose is to scale UI, the motivation section should be updated to reflect this. We can agree to allowing some touches on UI elements of the screen if touch events are offset and there's a minimum window size required for all integrations. We are strongly against any touch events being passed through to the app in PIP (1 in the comment above). For split screen (2), we can agree to some limited pass through of the touch events to the app, given a few restrictions. (2) changes to this:

If the change is a split-screen type change (cannot be handled by scaling the original video), then go through this flow. If the app does not support your custom split-resolution, either don't allow the user to put the app in split screen, or scale the video stream to a size that fits the window and use "black bars" to fill the rest of the window. Touches may be passed to the app in certain limited cases. (1) The window must have a width of at least 3" and a height of at least 3", (2) The touches must be offset for the developer so that 0,0 continues to be in the top-left corner of the app window. If these requirements are not met, either a system menu / buttons should be displayed when selected, or the selection should bring the user immediately back to the full-screen app.

[dboltovskyi]

8.
In order to resolve this point we propose to introduce new VIDEO_STREAMING_OPTIONS system capability type:

  <enum name="SystemCapabilityType">
    ...
+    <element name="VIDEO_STREAMING_OPTIONS"/>
  </enum>

And add new videoStreamingOptions array parameter in SystemCapability struct:

  <struct name="SystemCapability">
    ...
+    <param name="videoStreamingOptions" type="VideoStreamingCapability" mandatory="false" array="true">
  </struct>

HMI would be able to pass all supported VSCs within this new parameter.
Mobile application then would be able to obtain all available VSCs through GetSystemCapability request/response with systemCapabilityType=VIDEO_STREAMING_OPTIONS

Name VIDEO_STREAMING_OPTIONS is just an example and may by changed to more suitable one.

[dboltovskyi]

9.
We agree that motivation section needs to be updated to reflect possibility to scale UI.

We may add a specific section upon guidelines for HMI in terms of how HMI should create the projection areas and how touch events needs to be handled. However we suppose that is not in scope of this proposal. That has to be a broader topic for discussion.
Current proposal related to VSC changes and aspects how Mobile application supports the VSC provided by head unit.
The way how head unit uses that VSC is up to OEM.

[dboltovskyi]

6.
We like PM suggestion and would like to clarify a few items:
1) We assume Core would have to transfer OnAppCapabilityUpdated notification to HMI.
2) HMI would need to match somehow it's available system VSCs against received app's supported VSCs.

In order to simplify this process we would propose to use existing SystemCapabilityType enum and VideoStreamingCapability struct in new notification:

  <function name="OnAppCapabilityUpdated" functionID="OnAppCapabilityUpdatedID" messagetype="notification" since="x.x">
      <param name="appCapability" type="AppCapability" mandatory="true" />
  </function>

  <struct name="AppCapability" since="x.x">
      <param name="appCapabilityType" type="SystemCapabilityType" mandatory="true" />
      <param name="videoStreamingOptions" type="VideoStreamingCapability" mandatory="false" array="true" />
  </struct>

[theresalech]

During the 2020-05-12 meeting, the author noted that they will be posting information regarding the iOS implementation (item 3) once available, they agree with the PM’s stance on item 6, and they have shared their responses to items 8 and 9 on the review issue. The PM asked that responses to items 3 and 6 also be posted to the review issue for greater Steering Committee visibility. The Steering Committee voted to keep this proposal in review to allow the author and PM more time to discuss the open items on the review issue.

[dboltovskyi]

@theresalech Please notice we've posted response for 6 with some suggestions and hence items 6, 8 and 9 awaiting PM's review.

[theresalech]

Hi @dboltovskyi, thank you! Once item 3 has been answered by the Luxoft team, we will respond to all open items collectively.

[dboltovskyi]

@theresalech Thank you for update.

[theresalech]

The Steering Committee voted to keep this proposal in review, to allow time for the author to comment with changes required for iOS implementation (open item 3), and for the PM and SDLC members to review and respond.

[yoooriii]

Hello Steering Committee members, let me introduce iOS changes (following the point 3)

Objective-C classes and methods to update:

SDLCarWindow

In the following method the client app view gets resized to self.streamManager.videoScaleManager.appViewportFrame

- (void)sdl_applyDisplayDimensionsToViewController:(UIViewController *)viewController {
    const CGRect appFrame = self.streamManager.videoScaleManager.appViewportFrame;
    ...<app state and size validation logic>...
    viewController.view.frame = appFrame;
    viewController.view.bounds = appFrame;

    SDLLogD(@"Setting CarWindow frame to: %@", NSStringFromCGSize(appFrame.size));
}

Take for instance the following capabilities JSON with the following parameters:

{
  "prefferedResolution" : {
    "resolutionWidth": 800,
    "resolutionHeight": 380,
  },
  scale: 2,
  …<protocol, codec, etc are dropped>…
}

Display size: 800x380
Scale factor: 2.0
View size: (800x380)/2.0 ==> (400x190)

The method syncFrame generates screenshots 15 times a second (it is adjustable value)

- (void)syncFrame {
    //...<validate state parameters logic>...
    UIImage *screenshot = nil;
    const CGRect bounds = self.streamManager.videoScaleManager.appViewportFrame;
    UIGraphicsBeginImageContextWithOptions(bounds.size, YES, 1);
    CGContextRef context = UIGraphicsGetCurrentContext();
    switch (self.renderingType) {
        case SDLCarWindowRenderingTypeLayer: {
            [self.rootViewController.view.layer renderInContext:context];
        } break;
        case SDLCarWindowRenderingTypeViewAfterScreenUpdates: {
            [self.rootViewController.view drawViewHierarchyInRect:bounds afterScreenUpdates:YES];
        } break;
        case SDLCarWindowRenderingTypeViewBeforeScreenUpdates: {
            [self.rootViewController.view drawViewHierarchyInRect:bounds afterScreenUpdates:NO];
        } break;
    }
    screenshot = UIGraphicsGetImageFromCurrentImageContext();
    UIGraphicsEndImageContext();

    CGImageRef imageRef = screenshot.CGImage;
    CVPixelBufferRef pixelBuffer = imageRef ? [self.class sdl_createPixelBufferForImageRef:imageRef usingPool:self.streamManager.pixelBufferPool] : nil;
    if (pixelBuffer != nil) {
        const BOOL success = [self.streamManager sendVideoData:pixelBuffer];
        if (!success) {
            SDLLogE(@"Video frame will not be sent because the video frame encoding failed");
        }
        CVPixelBufferRelease(pixelBuffer);
    } else {
        SDLLogE(@"Video frame will not be sent because the pixelBuffer is nil");
    }
}

SDLStreamingVideoLifecycleManager

- (void)resumeVideo {
    self.shouldAutoResume = NO;

    if (self.protocol == nil) {
        SDLLogV(@"No session established with head unit. Cannot continue video.");
        return;
    }

    if (self.isVideoSuspended) {
        [self.videoStreamStateMachine transitionToState:SDLVideoStreamManagerStateReady];
    } else {
        [self sdl_startVideoSession];
    }
}

- (void)suspendVideo {
    if (!self.protocol) {
        SDLLogV(@"No session established with head unit. Cannot suspend video.");
        return;
    }

    [self.touchManager cancelPendingTouches];

    if (self.isVideoConnected) {
        [self.videoStreamStateMachine transitionToState:SDLVideoStreamManagerStateSuspended];
    } else {
        [self sdl_stopVideoSession];
    }
}

Request Video Capabilities and subscribe for SDLSystemCapabilityTypeVideoStreaming notifications
the callback is: -sdl_displayCapabilityDidUpdate:

- (void)didEnterStateVideoStreamStarting {
    SDLLogD(@"Video stream starting");

    __weak typeof(self) weakSelf = self;
    [self sdl_requestVideoCapabilities:^(SDLVideoStreamingCapability * _Nullable capability) {
        SDLLogD(@"Received video capability response, Capability: %@", capability);

        [weakSelf sdl_applyVideoCapability:capability];

        // Apply customEncoderSettings here. Note that value from HMI (such as maxBitrate) will be overwritten by custom settings.
        for (id key in weakSelf.customEncoderSettings.keyEnumerator) {
            weakSelf.videoEncoderSettings[key] = [weakSelf.customEncoderSettings valueForKey:key];
        }

        if (weakSelf.dataSource != nil) {
            SDLLogV(@"Calling data source for modified preferred resolutions");
            weakSelf.preferredResolutions = [weakSelf.dataSource resolutionFromHeadUnitPreferredResolution:weakSelf.preferredResolutions.firstObject];
            SDLLogD(@"Got specialized video resolutions: %@", weakSelf.preferredResolutions);
        }

        [weakSelf.systemCapabilityManager subscribeToCapabilityType:SDLSystemCapabilityTypeVideoStreaming withObserver:weakSelf selector:@selector(sdl_displayCapabilityDidUpdate:)];
        [weakSelf sdl_sendVideoStartService];
    }];
}

Note here the new introduced flag: shouldAutoResume
Which indicates that once suspended the video streaming must resume again.

- (void)sdl_displayCapabilityDidUpdate:(SDLSystemCapability *)systemCapability {
    SDLVideoStreamingCapability *videoCapability = systemCapability.videoStreamingCapability;
    SDLLogD(@"Video capabilities notification received: %@", videoCapability);

    self.videoStreamingCapability = videoCapability;
    self.shouldAutoResume = YES;
    dispatch_async(dispatch_get_main_queue(), ^{
        [self suspendVideo];
        // videoStreamingCapability will be applied later when video resumes
    });
}

If shouldAutoResume set to YES (as in the previous method) then the following method auto resume video streaming

- (void)didEnterStateVideoStreamSuspended {
    SDLLogD(@"Video stream suspended");
    [self sdl_disposeDisplayLink];
    if (self.shouldAutoResume) {
        self.shouldAutoResume = NO;
        dispatch_async(dispatch_get_main_queue(), ^{
            if (self.videoStreamingCapability) {
                [self sdl_applyVideoCapability:self.videoStreamingCapability];
            }
            [self resumeVideo];
        });
    }
    [[NSNotificationCenter defaultCenter] postNotificationName:SDLVideoStreamSuspendedNotification object:nil];
}

[yoooriii]

Known limitations and additional info. When streaming video the SDL library uses the following code:

    switch (self.renderingType) {
        case SDLCarWindowRenderingTypeLayer: {
            [self.rootViewController.view.layer renderInContext:context];
        } break;
        case SDLCarWindowRenderingTypeViewAfterScreenUpdates: {
            [self.rootViewController.view drawViewHierarchyInRect:bounds afterScreenUpdates:YES];
        } break;
        case SDLCarWindowRenderingTypeViewBeforeScreenUpdates: {
            [self.rootViewController.view drawViewHierarchyInRect:bounds afterScreenUpdates:NO];
        } break;
    }

Using this approach It is impossible to capture metal based layers (*1) such an attempt results in a black square instead of expected image.

(*1) _Metal based layers include but not limited to video player layer, SCNView and Scene framework, etc._

[theresalech]

The Steering Committee voted to keep this proposal in review, to allow time for the PM to respond to the latest comments from the author.

[joeljfischer]

Hi @dboltovskyi, @kostyaBoss, @yoooriii please see our responses to 3, 6, 8, and 9.

3. Please note that this is not a code review. This must be considered example code and will be required to go through a full code review when the PR is put to Github.

Android:
We can't change public methods without a breaking change (stopVideoService). It is also unnecessary. These changes could occur in the VideoStreamingManager itself. Simply using the service listener once the service is stopped, if the flag is set in the manager to restart the service it does so then.

• I think adding a new state to the StreamingStateMachine PAUSED makes sense
• In the SdlRemoteDisplay adding a new method as a callback to developers like onResizeView that the developers can override would be helpful. I would imagine it returned a boolean to state if they wish for the resizing to continue or not.

iOS:
There are many issues with the presented iOS code. I feel that there must have been a major miscommunication. The goal was to see the public API changes and a description of other changes that may need to take place.

a. You provide no public APIs. That is, given the iOS code, it is impossible for the developer to actually support a subset of the head unit's provided video streaming capabilities. This was the primary point of reviewing iOS code.

b.

SDLCarWindow

The code you provide is effectively unchanged from the code in the library and was unnecessary to provide. If there are differences that I missed, please point them out more effectively.

c.

SDLStreamingVideoLifecycleManager

i.
You provided new private methods but did not indicate how or why they would be used. The library already has ways to suspend and resume video streaming (e.g. if the app goes into the background on the phone). You have not explained why your new methods are necessary or why they exist.

ii.

Request Video Capabilities and subscribe for SDLSystemCapabilityTypeVideoStreaming notifications

This is already done, unless you mean your VIDEO_STREAMING_OPTIONS type, which we dispute is necessary, see above. It would be better to roll it all into the existing type rather than to have to subscribe to two different types in this manager. That will become complicated.

iii.

Note here the new introduced flag: shouldAutoResume
Which indicates that once suspended the video streaming must resume again.

What cases would this be false? We always want to autoResume, right?

iv.

- (void)sdl_displayCapabilityDidUpdate:(SDLSystemCapability *)systemCapability {

Why are you checking the videoCapabilities in the displayCapabilityDidUpdate method? That doesn't seem to be correct.

v.

• (void)didEnterStateVideoStreamSuspended

This code doesn't make sense. When the stream is suspended it should not be immediately resumed like this. Then the autoResume flag is turned to false, though it doesn't explain why. Suspension can happen for more reasons than because the video capabilities are changing.

vi.

Using this approach It is impossible to capture metal based layers (*1) such an attempt results in a black square instead of expected image.

This is entirely irrelevant to this proposal. If you want to change anything about this, open a new proposal.

6. Agree to the added points for OnAppCapability flow but should follow original naming proposed by the project maintainer. This would including the capability remaining VideoCapabilities as discussed in point 8.

We do not want to create a subset of SystemCapabilitiy enum values that only apply to app capabilities or a subset of system capabilities. Otherwise we would simply continue to use all of SystemCapability RPCs and structs.

8. I believe adding this as a param to the already existing capability will make the most sense. There should be no issue including an array of objects based on itself. It also ensures apps only have to handle one capability and all capabilities related to that feature are included in a single struct. This will help prevent cases of race conditions.

    <struct name="VideoStreamingCapability" since="4.5">
        <!- Existing params ->
        <param name="additionalVideoStreamingCapabilities" type="VideoStreamingCapability" array="true" minvalue="1" maxvalue="99" mandatory="false" since="X.X">
        </param>
    </struct>

9. We disagree, we feel that guidelines on how a feature should be used are in-scope to a proposal. If guidelines are not provided, OEMs could create conflicting implementations that app developers will have to handle themselves.

[kostyaBoss]

Hi @joeljfischer , @theresalech! I've created fork with specific branch where code of POC for this proposal could be found. Using this link, you can check all the changes for Android platofrm, I believe this would help us to avoid miscommunication and improve efficiency. There are still things to do (both technical cleanup and additional logic implementation), but general implementation of feature introduced. Please, let me know, if there would be additional concerns

[yoooriii]

Dear Steering Committee members I have just created a POC. In this POC I implemented the proposed functionality for iOS platform. It was tested on core 6.1.0. Please take a look at the changes proposed. In this POC I updated both the SDL library itself and 'SmartDeviceLink-Example-ObjC' project to use and test the proposed functionality. (Please note the SWIFT example project did not change and may or may not work).

The POC is available at:
yoooriii/sdl_ios/sdl0296_upd_video_stream_cap_

The proposed change does not affect the public API though it adds a few new methods to the API.

b). SDLCarWindow

- (void)updateVdeoStreamingCapability:(SDLVideoStreamingCapability *)videoStreamingCapability {
     [self sdl_applyDisplayDimensionsToViewController:self.rootViewController];
 }

c). SDLStreamingVideoLifecycleManager
In the method didEnterStateVideoStreamStarting it subscribes for notifications from SDLSystemCapabilityManager instead of sending a capability request.

- (void)didEnterStateVideoStreamStarting {
     [self.systemCapabilityManager unsubscribeFromCapabilityType:SDLSystemCapabilityTypeVideoStreaming withObserver:self];
     [self.systemCapabilityManager subscribeToCapabilityType:SDLSystemCapabilityTypeVideoStreaming withObserver:self selector:@selector(sdl_displayCapabilityDidUpdate:)];
 }

i). In the new implementation when video capability changes the SDL library closes the current video session and opens a new video session when doing reconnect the SDL lib renegotiates video parameters with the head unit.

ii). Please see the above method didEnterStateVideoStreamStarting, it subscribes for capability change.

iii). The new introduced flag shouldAutoResume is set to NO by default and it becomes YES when new video capabilities arrive and the SDL library is in state Ready SDLVideoStreamManagerStateReady. Maybe the name shouldAutoResume is ambiguous in this context if such case I am ready to rename it.

iv). SDLStreamingVideoLifecycleManager subscribes for notifications from SDLSystemCapabilityManager instead of sending a capability request. And as soon as either current or new updated capabilities arrive the method sdl_displayCapabilityDidUpdate gets called.

v). I agree it was a misconception on my part. Video streaming does not suspend but disconnects first and then reconnects once again. The video streaming state machine goes the following states: SDLVideoStreamManagerStateReady --> SDLVideoStreamManagerStateShuttingDown --> SDLVideoStreamManagerStateStopped and after that video streaming restarts: SDLVideoStreamManagerStateStopped --> SDLVideoStreamManagerStateStarting --> SDLVideoStreamManagerStateReady. The state SDLVideoStreamManagerStateSuspended is not used in this case.

vi). Yes sure it is not relevant though since the client app can be of any kind I thought it was worth mentioning. For instance video player will not work out of the box in comparison with the android counterpart.

[dboltovskyi]

1. 
   We're Ok with this approach if a recursion wouldn't lead to an issues.
   One small update we would like to have maxvalue="100"

[dboltovskyi]

6.
Taking item 8. into account please suggest our understanding is correct:

  <function name="OnAppCapabilityUpdated" functionID="OnAppCapabilityUpdatedID" messagetype="notification" since="x.x">
      <param name="appCapability" type="AppCapability" mandatory="true" />
  </function>

  <struct name="AppCapability" since="x.x">
      <param name="capabilityType" type="SystemCapabilityType" mandatory="true" />
      <param name="videoStreamingCapability" type="VideoStreamingCapability" mandatory="false" />
  </struct>

[kshala-ford]

Sorry for jumping into this discussion so late but as I support two of the major mobile navigation app partners I thought my feedback would be valuable.

First of all I highly recommend to design this feature with as little effort as possible especially for the app partner side. Generally I see some good use cases of informing Core about app capabilities. In this case I have some concerns. This issue has so many comments I couldn't figure out if the current suggestion is to have the app send an array of supported resolutions based on a list from the system or out of nowhere. In both cases I don't think the app developer can code some logic to properly answer to this list. Both of my app partners are not hesitant about different resolutions. Sure a high resolution is preferred but their major concern is the aspect ratio. One app partner has major issues with very wide screen resolutions because UI elements which are "docked" to the lower part of the app's view overlap with UI elements from the upper part.

Furthermore if the app developer is using the library to automate video streaming then we should do our best to also automate this feature. For iOS the CarWindow should straight change the view's bounds and continue capturing and encoding the window.

Instead of sending AppCapability my suggestion would be to provide the additional resolutions to the app in the VideoStreamingCapability. The new resolutions should be named so that it is known for what the resolution is used for. A good example of names was already mentioned here:

+<enum name="VideoStreamingResolutionTypes">
+<element name="DEFAULT"/>
+<element name="FULL_SCREEN"/>
+<element name="PICTURE_IN_PICTURE"/>
+<element name="PREVIEW"/>
+<element name="SPLIT_SCREEN"/>
+<element name="COLLAPSED_VIEW"/>
+</enum>

+ <struct name="AdditionalResolutions"> <!-- There's for sure a better name :) -->
+    <param name="type" type="VideoStreamingResolutionTypes" mandatory="true"/>
+   <param name="resolution" type="ImageResolution" mandatory="true"/>
+</struct>

<struct name="VideoStreamingCapability" since="4.5">
        <description>Contains information about this system's video streaming capabilities.</description>
        <param name="preferredResolution" type="ImageResolution" mandatory="false">
            <description>The preferred resolution of a video stream for decoding and rendering on HMI.</description>
        </param>
+    <param name="additionalResolutions" type="AdditionalResolutions" mandatory="false" array="true" />

This way the system can provide information what other types it supports. Each type has aspects like picture in picture should basically show the app next to another UI view but everything is shown very small as the content is scaled. Preview however is similar with regards to size on the GUI but the scaling should allow interaction with view controls. Splitscreen may cause a ridiculous aspect ratio etc.

Apps can choose to support different types and exclude some they don't support. They do this by configuring the SDL manager. The manager however doesn't send an RPC like AppCapability. Instead my suggestion is to use the protocol spec.

According to Start Video Service there should be an additional tag in the payload called supportedResolutionTypes which populates with the developer's configuration.

Tag Name | Type | Introduced | Description
-- | -- | -- | --
supportedResolutionTypes | String Array | X.Y | Indicates which resolution types the app supports allowing the HMI whether or not it should provide the possibility to switch the app UI to fullscreen, splitscreen etc. |

Based on the supported types the HMI can offer user control to move the app to full screen, splitscreen etc. If the app doesn't want to be shown in small view ("picture in picture or "preview") the HMI shouldn't show the app this way.

If the HMI wants to change the type to e.g. Full screen upon user control, the system should send a control frame of the video service to the app The control frame payload includes one tag called resolutionType and also two tags for width and height.

With this control frame the SDL library can suspend the remote display/car window, stop the video encoder, resize views and restart the video encoder again. Once everything is complete the SDL library should also send the same control frame to the system before it sends the first video data. This is very very important for the UI/UX so that the HMI knows exactly which byte from the protocol session are related to the new resolution. Otherwise the HMI and the video stream are not aligned and present the app in a weird aspect/scale. At all time the video service session must remain open. it must not be stopped and restarted otherwise a huge delay is added for TLS handshake and encryption init.

I know this suggestion is vastly different from what was suggested before and my suggestion is not fleshed out but I am worried that the conversation is not including the protocol level. I don't think we can resolve everything in the RPC level.

[okandul]

9.

I just want to clarify two topics:
9a. Is it correct the HMI guidelines is the recommendation and describes the reference implementation presented in the Open Source version of SDL? If no, who and how work with the guidelines?

9b. Regarding the touch events for the scaled controls. As I understood, the idea of the proposal is to have similar behavior as described here https://developer.android.com/guide/topics/ui/picture-in-picture. If so, the HMI should prepare the "infrastructure" for the app. Everything else needs to be managed by the mobile app. The "infrastructure" (minimum window size, etc.) might be described in the guidelines but shouldn't limit the app itself.

[theresalech]

The Steering Committee voted to keep this proposal in review until our next meeting, to allow the author time to respond to the latest comments on the review issue, and to allow the PM and other SDLC members to review and respond as well.

[theresalech]

Please find collective feedback from the PM below.

3.

The proposed change does not affect the public API though it adds a few new methods to the API.

We're struggling to comprehend what is being said here. How does something add a few methods but not change the public API? How can we ask devs to declare support / lack of support for various resolutions without any public API changes?

The majority of what has been posted here is unnecessary and not especially helpful for the purposes of the proposal review. We understand that this can be implemented into the library. The discussion is over the public APIs and how resolution switching will occur. The rest, in my mind, are implementation details that can be sorted out during implementation.

6. We do not want to create a subset of SystemCapabilitiy enum values that only apply to app capabilities or a subset of system capabilities. Otherwise we would simply continue to use all of SystemCapability RPCs and structs. We'd recommend:

  <function name="OnAppCapabilityUpdated" functionID="OnAppCapabilityUpdatedID" messagetype="notification" since="x.x">
      <param name="appCapability" type="AppCapability" mandatory="true" />
  </function>

  <struct name="AppCapability" since="x.x">
      <param name="appCapabilityType" type="AppCapabilityType" mandatory="true" />
      <param name="videoStreamingCapability" type="VideoStreamingCapability" mandatory="false" />
  </struct>

   <enum name="AppCapabilityType" since="x.x">
        <description>Enumerations of all available app capability types</description>
        <element name="VIDEO_STREAMING"/>
    </enum>

8. 👍

9a. Yes, there are recommendations present, but there are also requirements for HMI integrators.

9b. Every platform provides some restrictions on applications, whether Android, iOS, CarPlay, Android Auto., including SDL. We provide a number of restrictions on OEM UX with SDL (see templates, etc.). If we provide no boundaries for apps, the appeal to integrate SDL is lessened because the amount of work the app developer needs to do grows exponentially. Therefore, stating that everything is up to the app developer just isn’t true in an SDL context, nor in any other context. If Livio’s proposed restrictions are not followed, we enter a place where app developers don’t know what to expect. Some head units may implement a 1"x1" PIP and pass through touches, which requires the app dev to show buttons, which is unusable to the user. Another head unit might absorb those touches. Then the dev displays buttons even though touches can’t occur. We believe that our proposed restrictions are reasonable to provide consistent app developer expectations and a generally good HMI user experience.

---

@kshala-ford

This issue has so many comments I couldn't figure out if the current suggestion is to have the app send an array of supported resolutions based on a list from the system or out of nowhere.

It was the former.

Sure a high resolution is preferred but their major concern is the aspect ratio.

This has been Livio's concern as well.

Instead of sending AppCapability my suggestion would be to provide the additional resolutions to the app in the VideoStreamingCapability.

This is the current idea, but the app needs a way to declare whether or not they support those different resolutions, so that the head unit can disable things like the PIP button, etc. Otherwise, the HU will display UI that the app does not support. This was deemed to be poor UX.

VideoStreamingResolutionTypes

I don't think that this enum is helpful. What's important is the resolution / scale, etc. A split screen display could be top/bottom or left/right, for example. A dev declaring that they support certain types like you've defined could have wildly different sizes and aspect ratios.

Your API here is close to the current idea, but the current idea is to embed additional VideoStreamingCapability in the top-level VideoStreamingCapability so that information like scale and actual size (in cm / in) can be sent to the app developer.

However, if these types were standardized (aspect ratios, sizes within ranges, etc.), then this would be more helpful.

If the HMI wants to change the type to e.g. Full screen upon user control, the system should send a control frame of the video service to the app The control frame payload includes one tag called resolutionType and also two tags for width and height.

Is this an ask to add a new protocol control service packet or to modify StartService? The current idea is to use the StartService packet to start the new resolution sizes, etc. The reasoning behind this is that there is no current way to update the resolution without ending the service and starting the service again. If we want to add that capability, then that would be an improvement.

I know this suggestion is vastly different from what was suggested before and my suggestion is not fleshed out but I am worried that the conversation is not including the protocol level.

The protocol spec has been discussed at some point before, and your ideas aren't totally different from the current agreement.

[kostyaBoss]

Hi @dboltovskyi, @kostyaBoss, @yoooriii please see our responses to 3, 6, 8, and 9.

1. Please note that this is not a code review. This must be considered example code and will be required to go through a full code review when the PR is put to Github.

Android:
We can't change public methods without a breaking change (stopVideoService). It is also unnecessary. These changes could occur in the VideoStreamingManager itself. Simply using the service listener once the service is stopped, if the flag is set in the manager to restart the service it does so then.

* I think adding a new state to the `StreamingStateMachine` PAUSED makes sense

* In the `SdlRemoteDisplay` adding a new method as a callback to developers like `on�ResizeView` that the developers can override would be helpful. I would imagine it returned a boolean to state if they wish for the resizing to continue or not.

@joeygrover POC for Android updated with mentioned items:

• Added Stopped state
• Implemented callback with updated view parameters for third-party developers to give more control on this event
• Removed dependencies on stopVideoService method and encapsulated most of logic in VideoStreamManager using ServiceListener.

I didn't change branch of the PoC, all changes can be checked using this link

[kshala-ford]

@ PM thanks for the response. I went through the proposal again and talked to the author and I agree my suggestions are close to the proposed solution. Therefore please disregard my comment. Instead I want to suggest something with regards to issue 3 of how to add new public APIs for the app developers.

3.

We're struggling to comprehend what is being said here. How does something add a few methods but not change the public API? How can we ask devs to declare support / lack of support for various resolutions without any public API changes?

I don't think an app developer will be able to manually distinguish supported resolutions on their own. Also the SDL managers meant to take over as much responsibility as possible to simplify SDL integration. Instead of having the app developer to read the available resolutions, magically calculate which ones it supports and reply with supported resolutions, the SDL libraries should be responsible for this. The libs will require to get additional params from the app dev through the streaming configuration or the builder. I must admit I am not very familiar with Android video streaming but the point is that identifying supported resolutions can be done if the app provides the following information:

• aspect ratio min/max (relative value (longer edge divided by shorter edge))
• optionally width min/max (in points (independent of scale) not in pixels (dependent of scale))
• optionally height min/max (in points (independent of scale) not in pixels (dependent of scale))

Following iOS example should present how the app could configure the SDL manager with the supported ranges.

@interface SDLSupportedStreamingRange
// The minimum supported aspect ratio, Min value is 1
@property (nonatomic) NSUInteger minimumAspectRatio;
// The maximum supported aspect ratio.
@property (nonatomic) NSUInteger maximumAspectRatio;
// The minimum resolution to support
@property (nonatomic, nullable) SDLImageResolution *minimumResolution;
// The maximum resolution to support
@property (nonatomic, nullable) SDLImageResolution *maximumResolution,
@end

@interface SDLStreamingMediaConfiguration

@property (nonatomic, nullable) SDLSupportedStreamingRange supportedLandscapeStreamingRange;
@property (nonatomic, nullable) SDLSupportedStreamingRange supportedPortraitStreamingRange;

+ (instancetype)autostreamingSecureConfigurationWithInitialViewController:(UIViewController *)initialViewController 
    supportedStreamingRange:(SDLSupportedStreamingRange *)supportedStreamingRange;

+ (instancetype)autostreamingSecureConfigurationWithInitialViewController:(UIViewController *)initialViewController 
    supportedLandscapeStreamingRange:(SDLSupportedStreamingRange *)supportedLandscapeStreamingRange 
    supportedPortraitStreamingRange:(SDLSupportedStreamingRange *)supportedPortraitStreamingRange;

@end

From the two big navigation app partners I support, one prefers portrait views while the other prefers landscape (ideally square views). Therefore this information should be provided two times for portrait and for landscape views. Sure when constructing the SDL manager a convenience function or constructor should allow specifying information once which the library then uses for landscape and portrait.

Once the library receives the list of available resolutions it can distinguish the supported resolutions quite easily.

1. Loop through all available resolutions
2. Identify if the resolution is landscape/portrait
3. Is the aspect ratio within the range of the configured aspect ratio?
4. Is the resolution within the range of the configured resolution?

After all this suggestion is only a small addition to the suggested negotiation flow where "App" should be separated into "App" and "Library". Also the information we request from the app developer should already exist or at least partly. The app's design teams should already know the aspect ratios where the app appears okay without UI elements to overlap. Same appears for minimum resolution.

[dboltovskyi]

1. 
   Thank you for clarification. We agree with this PM's suggestion.

[dboltovskyi]

9.
Taking all the discussion into account we agree to add suggested by PM section to HMI Integration Guidelines .

[yoooriii]

I would like to clarify the following statement:

The proposed change does not affect the public API though it adds a few new methods to the API.

Here I mean backward compatibility, old apps can use API as they did, "no change in API".
New apps can take advantage of "new added methods in API".

Thank you.

[joeljfischer]

3.

Also the SDL managers meant to take over as much responsibility as possible to simplify SDL integration. Instead of having the app developer to read the available resolutions, magically calculate which ones it supports and reply with supported resolutions, the SDL libraries should be responsible for this.

They would do it in a similar way that we would, by calculating aspect ratios, but I do agree that we may be able to assist them.

I think that if we have to actual sizes that will be used (in inches / centimeters), that may be necessary information for the developer as well. For example, a developer may want to display a 200x200 point screen if the screen is 4" diagonal, but not if it's 1" diagonal. I think we could pass:

• aspect ratio min/max (relative value (longer edge divided by shorter edge))
• resolution min/max in points
• minimum scale
• minimum diagonalScreenSize in inches

This would allow them to provide all the important information without confusion. They could say "we don't support anything at scale 1.0", "or anything less than 2 inches."

However, even this may not be enough. We still need to somehow tell the developer when the streaming size changes and not just change the bounds on them (though we can do that for them too). For example, a 1 inch diagonal screen should probably not have buttons on it because they'd be too small to touch, so the dev needs to re-format their interface for that size (or for portrait vs. landscape, etc.).

So I think this is a good start, but there still needs to be callbacks to the dev informing them of sizing changes. Alternatively, if we just change the bounds on them, are there iOS / Android system calls we can tell developers to use to update their interface? That should be documented in the proposal as well.

[kshala-ford]

@joeljfischer

3.1. What information do we need from app dev?

Another parameter for diagonal screen size could make sense. PointsPerInch could be an alternative allowing easier calculation but it's basically the same information.

We discussed if scale is needed and we believe the resolution in points with screen size is sufficient for the developer to know the size of the app being presented. The intention of scale is to make the app appear the same way regardless of the display's pixel density. It's an SDL internal information and abstracted from the developer.

From our point of view scale is not required and would rather cause confusion to the developer.

3.2. How do we notify the app dev on changes?

I know both my app partners have designed the view dynamically so that the UI elements like buttons rearrange or (dis)appear automatically when changing the view size. However, not every app developer does that. Also the screen size could potentially change but the resolution remains the same, though I couldn't know if there's a use case for this. Notifying the app developer through a callback makes sense and wouldn't hurt anyways.

[joeljfischer]

3.1. I think that between the diagonal screen size and the resolution in points it should be okay.

3.2. 👍 I want to make sure also for the app partners who change the view dynamically that the view is resized in such a way that their dynamic re-sizing activates, if that makes sense. Or, if we can't trigger those system calls, they will need to hook it up to our own callbacks.

[theresalech]

It was noted that items 6 and 9 have been agreed upon, but details of item 3 are still in discussion. It was requested that instead of a POC for item 3, code samples and explanations for the Public APIs (3.1) and a way to describe how the resolution switching will work (3.2) are all that’s necessary. The Steering Committee voted to keep this proposal in review to allow the author and Project Maintainer to work through the details for item 3.

[joeljfischer]

Hi, I'd like to try to document the agreed upon revisions up to this point. Please let me know if I missed anything or got something wrong:

1. Author to Revise. Remove APP_SERVICES, add DISPLAYS regarding HMI OnSystemCapabilityUpdated. Note that this may no longer be necessary given the updates to 2 and 6.
2. No Action Required. Previously agreed upon solution was voided by the discussion on 6.
3. Open. Still working out the details of the iOS / Android manager-level public API changes for developers to integrate.
4. No Action Required. Concerns about scalability for app developers has moved to point 3/6.
5. Open. Descriptions of how this proposal will handle protocol video start service / ACK with new stream resolutions. Will there be a new control service packet? Will the app re-send StartService / Core send StartServiceACK while already connected in order to update?

6. Author to Revise. Revise to use new flow to pass potential video capabilities.

   <function name="OnAppCapabilityUpdated" functionID="OnAppCapabilityUpdatedID" messagetype="notification" since="x.x">
       <description>A notification to inform SDL Core that a specific app capability has changed.</description>
       <param name="appCapability" type="AppCapability" mandatory="true">
           <description>The app capability that has been updated</description>
       </param>
   </function>
   
   <struct name="AppCapability" since="x.x">
       <param name="capabilityType" type="AppCapabilityType" mandatory="true">
           <description>Used as a descriptor of what data to expect in this struct. The corresponding param to this enum should be included and the only other param included.</description>
       </param>
       <param name="videoStreamingCapability" type="VideoStreamingCapability" mandatory="false">
           <description>Describes supported capabilities for video streaming </description>
       </param>
   </struct>
   
   <struct name="AppVideoStreamingCapability" since="x.x">
       <description>Contains information about this app's video streaming capabilities.</description>
       <param name="supportedResolutions" type="ImageResolution" mandatory="false" array="true">
           <description>Supported streaming resolutions</description>
       </param>
   </struct>
   
   <enum name="AppCapabilityType" since="x.x">
       <description>Enumerations of all available app capability types</description>
       <element name="VIDEO_STREAMING"/>
   </enum>
   
   <struct name="VideoStreamingCapability" since="4.5">
       <!- Existing params ->
       <param name="additionalVideoStreamingCapabilities" type="VideoStreamingCapability" array="true" minvalue="1" maxvalue="99" mandatory="false" since="X.X">
       </param>
   </struct>
   

   • Add updated flow chart with HMI <-> Core <-> App communications.
   • Core will have to transfer OnAppCapabilityUpdated information to HMI.
   • HMI will need to match available system VSCs against app's supported VSCs.
7. Author to Revise. Flows for existing apps that don't update and apps that only support some video capabilities will be provided.
   
   
   
   • Disable any kind of resolution switching (PIP, Split etc.) Only Full screen view would be available. This is current behavior which doesn't require any changes on HMI side.
   
   
   • Resize app's window as described in #983 (comment) in 1st example. This would require an implementation on HMI side.
   
   
8. No Action Required. See 6.

9. Author to Revise. Add HMI Guidelines:

   If the change is a PIP-type change, scale the current video stream to the size you desire without going through this flow. Touches should never be passed to the app. Either a system menu/buttons should be displayed when selected, or the selection should bring the user immediately back to the full-screen app.

   If the change is a split-screen type change (and cannot be handled by scaling the original video), then go through this flow. If the app does not support your custom split-resolution, either don't allow the user to put the app in split screen, or scale the video stream to a size that fits the window and use "black bars" to fill the rest of the window. Touches may be passed to the app in certain limited cases. (1) The window must have a width of at least 3" and a height of at least 3", (2) The touches must be offset for the developer so that 0,0 continues to be in the top-left corner of the app window. If these requirements are not met, either a system menu / buttons should be displayed when selected, or the selection should bring the user immediately back to the full-screen app.

   • Update motivation and proposed solution to mention scaling UI elements.

[yoooriii]

3.2. How do we notify the app dev on changes?

iOS part

I propose to add add a new delegate property to SDLStreamingVideoLifecycleManager and pass it in the configuration

SDLStreamingMediaConfiguration as a new property:

@property (weak, nonatomic, nullable) id<SDLStreamingVideoDelegate> streamingVideoDelegate;

Where SDLStreamingVideoDelegate implements following methods:

- (void)videoStreamingSizeDidUpdate:(CGSize)displaySize;

When SDL lib receives the struct SDLVideoStreamingCapability it must compare it with the client app provided supportedLandscapeStreamingRange and supportedPortraitStreamingRange and finds a match.
When a match found then the video streaming delegate will be informed with the method:
-videoStreamingSizeDidUpdate:(CGSize)size and video streaming service starts as usual.

https://github.com/yoooriii/sdl_ios/blob/21d56ad4d0122426319345d8685cf803776582a8/SmartDeviceLink/SDLStreamingVideoLifecycleManager.m#L709

[dboltovskyi]

3.2. _How do we notify the app dev on changes?_

Android part

Requied call back is implemented in PoC.
Please find details: https://github.com/kostyaBoss/sdl_java_suite/blob/poc/fork/video_streaming_with_capabilities_update/android/hello_sdl_android/src/main/java/com/sdl/hellosdlandroid/SdlService.java#L535

[dboltovskyi]

5.
We propose an approach of restarting the video service with new streaming parameters.
In this way we don't expect changes on a protocol level and retain existing negotiation sequence.

This is already implemented in both PoCs for Android and iOS.

[joeljfischer]

5. Your suggestion conflicts with @kshala-ford's comments here:

At all time the video service session must remain open. it must not be stopped and restarted otherwise a huge delay is added for TLS handshake and encryption init.

I agree with Kujtim here. We ought not stop and restart the service or this will lead to a poor user experience.

[dboltovskyi]

5.
We did an internal analysis and found out Stop/Start approach works fine.
As for TLS handshake it will run only once at the start of any service in protected mode (RPC, VIDEO or AUDIO). Once secure channel established it then used when starting a new service in protected mode. This is also applicable for service restart.

@kshala-ford Could you please confirm you're Ok with Stop/Start approach.

[kshala-ford]

Yes this is the case. The TLS handshake is performed only once. The handshake is not performed again when sending another StartService. The handshake authenticates the whole app transport connection, not every individual SDL protocol session.

@joeljfischer This was my major concern but the author already investigated here and explained that the handshake is performed only once. The effort to add new protocol frame types gives us probably a few milliseconds only. This is why I wrote

I went through the proposal again and talked to the author and I agree my suggestions are close to the proposed solution. Therefore please disregard my comment.

[kostyaBoss]

3.1 What information do we need from app dev?

Providing code snippets below to describe idea on high-level

iOS part

To support different screen resolutions the client App must configure SDLStreamingVideoLifecycleManager properly.
For this there were added 2 new properties to SDLStreamingMediaConfiguration class:

@property (strong, nonatomic, nullable) SDLSupportedStreamingRange *supportedLandscapeStreamingRange;
@property (strong, nonatomic, nullable) SDLSupportedStreamingRange *supportedPortraitStreamingRange;
`SDLSupportedStreamingRange` implemented as it was proposed earlier with a little change: 
@interface SDLSupportedStreamingRange : NSObject
// The minimum supported normalized aspect ratio, Min value is 1
@property (nonatomic, assign) float minimumAspectRatio;
// The maximum supported normalized aspect ratio, Min value is 1
@property (nonatomic, assign) float maximumAspectRatio;
// The minimum supported diagonal screen size in inches, defaults to 0 (0 matches any size)
@property (nonatomic, assign) float minimumDiagonal;
// The minimum resolution to support, it overrides .minimumAspectRatio
@property (nonatomic, assign) float minimumResolution;
// The maximum resolution to support, it overrides .maximumAspectRatio
@property (nonatomic, assign) float maximumResolution;
// Check if the argument is within the [.minimumResolution, .maximumResolution] range
- (BOOL)isImageResolutionInRange:(SDLImageResolution*)imageResolution;
// Check if the argument is within the [.minimumAspectRatio, .maximumAspectRatio] range
- (BOOL)isAspectRatioInRange:(float)aspectRatio;
<……>
@end

If the app does not provide either supportedLandscapeStreamingRange or supportedPortraitStreamingRange then SDLStreamingVideoLifecycleManager will take preferred resolution supported by the head unit.

The most important here is SDLStreamingVideoLifecycleManager which implements the match logic and chooses which resolution it should take from all resolutions supported by the head unit.
When it receives system capabilities in the callback:

- (void)sdl_displayCapabilityDidUpdate:(SDLSystemCapability *)systemCapability
it has to check all video streaming capabilities against the app provided min and max image resolutions:
- (SDLVideoStreamingCapability * __nullable)matchVideoCapability:(SDLVideoStreamingCapability*)videoStreamingCapability {
  if (!videoStreamingCapability) {
    return nil;
  }
  NSArray <SDLVideoStreamingCapability*>* allCapabilities = [videoStreamingCapability allVideoStreamingCapabilitiesPlain];
  if (!self.supportedLandscapeStreamingRange || !self.supportedPortraitStreamingRange) {
    // there is nothing to match, return any (1st) object
    return [allCapabilities firstObject];
  }
  for (SDLVideoStreamingCapability* nextCapability in allCapabilities) {
    SDLImageResolution *imageResolution = [nextCapability makeImageResolution];
    if ([self.supportedPortraitStreamingRange isImageResolutionInRange:imageResolution] ||
      [self.supportedLandscapeStreamingRange isImageResolutionInRange:imageResolution]) {
      return nextCapability;
    }
    const float diagonal = nextCapability.diagonalScreenSize.floatValue;
    if (0 < diagonal) {
      if (diagonal >= self.supportedPortraitStreamingRange.minimumDiagonal ||
        diagonal >= self.supportedLandscapeStreamingRange.minimumDiagonal) {
        return nextCapability;
      }
    }
    const float ratio = nextCapability.preferredResolution.normalizedAspectRatio;
    if (1 <= ratio) {
      if ([self.supportedPortraitStreamingRange isAspectRatioInRange:ratio] ||
        [self.supportedLandscapeStreamingRange isAspectRatioInRange:ratio]) {
        return nextCapability;
      }
    }
  }
  return nil;
}

The full iOS implementation is available in the following repo: GitHub

Android part

In SdlRemoteDisplay's implementations by developers

@Override
public Integer getMinSupportedWidth() {
    return 800;
}
@Override
public Integer getMinSupportedHeight() {
    return 600;
}
@Override
public Integer getMaxSupportedWidth() {
    return 1600;
}
@Override
public Integer getMaxSupportedHeight() {
    return 1200;
}
@Override
public double getMinScreenDiagonal() {
    return 6;
}
   @Override
   public Double getMinAspectRatio() {
     return 1.;
   }
   @Override
   public Double getMaxAspectRatio() {
     return 4.;
   }

In SdlRemoteDisplay

public abstract Integer getMinSupportedWidth();
public abstract Integer getMinSupportedHeight();
public abstract Integer getMaxSupportedWidth();
public abstract Integer getMaxSupportedHeight();
public abstract double getMinScreenDiagonal();
public abstract Double getMinAspectRatio();
public abstract Double getMaxAspectRatio();

In VideoStreamManager

List<VideoStreamingCapability> getSupportedCapabilities(
        Integer constraintHeightMax,
        Integer constraintHeightMin,
        Integer constraintWidthMax,
        Integer constraintWidthMin,
        Double constraintDiagonalMin,
        Double aspectRationMin,
        Double aspectRationMax
){
    List<VideoStreamingCapability> validCapabilities = new ArrayList<>();
    List<VideoStreamingCapability> allCapabilities = getMockedAllCapabilities();
    // get the first one - the Desired resolution to guarantee streaming will start
    validCapabilities.add(allCapabilities.get(0));
    for (VideoStreamingCapability capability : allCapabilities) {
        int resolutionHeight = capability.getPreferredResolution().getResolutionHeight();
        int resolutionWidth = capability.getPreferredResolution().getResolutionWidth();
        double diagonal = capability.getDiagonalScreenSize();
        if (diagonal < constraintDiagonalMin) {
            continue;
        }
        if (!(aspectRationMax > aspectRationMin && aspectRationMin > 0)) {
            if (constraintHeightMax == null && constraintHeightMin == null) {
                continue;
            }
        }
        if (resolutionHeight > 0 && resolutionWidth > 0 && constraintHeightMax != null && constraintHeightMin != null)
        {
            if (!(resolutionHeight > constraintHeightMin && resolutionHeight < constraintHeightMax)) {
                continue;
            }
            if (!(resolutionWidth > constraintWidthMin && resolutionHeight < constraintWidthMax)) {
                continue;
            }
        }
        validCapabilities.add(capability);
    }
    return validCapabilities;
}

Finally params are passed to getSupportedCapabilities method in VideoStreamManager, which has access to remoteDisplay
The full implementation could be found here

[dboltovskyi]

For convenience here is the summary of responses for Open items:

3.1. Proposed public API changes for iOS and Android: comment-643313592

3.2. Examples of notification on VSC changes for iOS: comment-642797352 and Android: comment-642852789

5. Proposed sequence for video streaming resolution switching comment-643288059 and comment-643292703

[theresalech]

@dboltovskyi, thank you! We are okay with your proposed sequence for item 5, and don't believe any further action is required for this item. For 3.1 and 3.2, we'd like to return this proposal for the revisions summarized in this comment, and have these code snippets added to the proposal as part of that revision. We may have additional feedback on the code snippets, but will provide those in the next round of review.

[theresalech]

The Steering Committee voted to return this proposal for revisions. The author will revise to include the agreed upon items (1, 6, 7, and 9) included in this comment, and to include the code snippets for iOS/Android manager-level public API changes (item 3). It was noted that there may be additional feedback on the public API changes in the next review, but reviewing these within the proposal will be clearer than across comments on the review issue. There was discussion about accepting this proposal with revisions, but it was determined that, given the number of changes requested and public API changes review noted above, returning this proposal for revisions is the best way to allow members to review the proposal as a whole before voting to accept.

[theresalech]

The author has updated this proposal to reflect the requested revisions, and the revised proposal is now under review until June 30, 2020. Exact changes made since the last version of this proposal was reviewed can be found in this merged PR: https://github.com/smartdevicelink/sdl_evolution/pull/1003.

[dboltovskyi]

10.
We propose to remove AppVideoStreamingCapability struct as it's not required (and not used) since whole VideoStreamingCapability struct is utilized for new additionalVideoStreamingCapabilities parameter.

[theresalech]

@dboltovskyi , after reviewing the revised proposal, we have minor comments about the iOS and Java Suite implementations (item 3):

3.
For iOS, we think that the property name streamingVideoDelegate can be simplified to just delegate.

For both, we'd rather see a naming like VideoStreamingRange as it is more descriptive that this is specific to video streaming, but also that it is simply a range and the context in which it is used will define if it is "supported" or not.

Additionally, we've responded to your new comment and provided another new comment (11) below:

10.
We disagree, and think it would be better to change the AppCapability struct to take a AppVideoStreamingCapability type in the videoStreamingCapability param. This would allow us to expand that type in the future if needed. It would mean that the supportedResolutions would need to match the resolutions sent by the head unit.

11.
We would like to propose changes for HMI Integration Guidelines: The HMI should stop displaying video to the user after resizing the window instead of waiting for the StopStream request from Core. Video would resume after the video service is restarted.

[atiwari9]

@theresalech

1. 
   It is better to provide entirety of VSC from app side as well rather than just ImageResolution. If you do want to use a new struct AppVideoStreamingCapability for scalability reasons, then i'd recommend to change supportedResolutions -> videoStreamingCapability (type=VideoStreamingCapability) with-in that struct.

[joeljfischer]

10.

@atiwari9 That was my initial thought as well, but I think it's unnecessary. All that the mobile side has to do is state which of the head unit's video capabilities it supports. Because there should not be multiple capabilities from the head unit with the same ImageResolution, the app declaring that it supports video capabilities with those resolutions should be enough, right?

[atiwari9]

@joeljfischer

1. Scale is also an important factor in determining the true resolution here. Now we can set some rules around ImageResolution being the calculated resolution after scale, but that will create additional computation on HMI side and add some possible error entry points. Since HMI is sending the supported resolutions in one format(all of VSC), then for simplicity it should receive the response back from app in same format. This would also save SDL managers some computation to trim the VSCs or recalculate resolution after applying scale etc..

[dboltovskyi]

@theresalech

3.
We agree with provided suggestion.

11.
We agree with proposed change.

[Sohei-Suzuki-Nexty]

11.

The HMI should stop displaying video to the user after resizing the window instead of waiting for the StopStream request from Core.

Why aren't you waiting for a StopStream request?
Since the state management is not clear, there are some concerns that problems will occur.

[theresalech]

@dboltovskyi @atiwari9 @Sohei-Suzuki-Nexty , please find responses and one new comment from the PM below. Thank you!

10.
After internal discussions, we believe we can use VideoStreamingCapability struct for the AppCapability feature. While each capability will have to be reviewed to see if they can be reused for both system and app capabilities, we do feel confident enough to reuse VideoStreamingCapability for both.

11.
The reason for that is to ensure that the video stream is not displayed to the user in a distorted format. The state flow would be "user attempts to resize" -> "video stops streaming / head unit sends OnAppCapabilityUpdated" -> "HMI resizes / waits for stop and start of video stream" -> "display new video stream"

12. HMI needs to be added as an impacted platform.

[Shohei-Kawano]

@theresalech -san
Thank you for reply.

1. In other words, after the user requests to attempts to resize, does the HMI mask (Mute?) the video without waiting for StopStream?

[joeljfischer]

@Sohei-Suzuki-Nexty

1. Correct, the HMI would simply stop displaying the incoming video to the user. The audio stream would continue as normal.

[theresalech]

We did not have a quorum present during the 2020-07-07 Steering Committee meeting, so voting on this proposal could not take place. This proposal will remain in review until 2020-07-14.

[theresalech]

@dboltovskyi, given the additional questions from SDLC members regarding Item 11, we'd like to add a bit more detail to the HMI Integration Guidelines, and suggest the following:

Add the following to the HMI Integration Guidelines: The HMI should stop displaying video to the user after resizing the window instead of waiting for the StopStream request from Core. Video would resume after the video service is restarted. See the following example state flow: "user attempts to resize" -> "hmi stops displaying video -> head unit sends OnAppCapabilityUpdated" -> "HMI resizes / waits for stop and start of video stream" -> "display new video stream".

HMI Integration Guidelines updates should include a flow chart documenting this behavior.

[dboltovskyi]

@theresalech We agree to add suggested notice to HMI Integration Guidelines.
Instead of creating a new diagram we would propose to extend existing 'Resolution Switching' diagram with additional actions.

[theresalech]

The Steering Committee voted to accept this proposal with the following revisions:

• (Item 3) For iOS, change streamingVideoDelegate to delegate. For Both iOS and Java Suite, use VideoStreamingRange.
• (Item 10) Use VideoStreamingCapability struct for the AppCapability feature. While each capability will have to be reviewed to see if they can be reused for both system and app capabilities, we do feel confident enough to reuse VideoStreamingCapability for both.
• (Item 11) Add the following to the HMI Integration Guidelines, and extend the Resolution Switching diagram to document this behavior.
  > The HMI should stop displaying video to the user after resizing the window instead of waiting for the StopStream request from Core. Video would resume after the video service is restarted. See the following example state flow: "user attempts to resize" -> "HMI stops displaying video" -> "head unit sends OnAppCapabilityUpdated" -> "HMI resizes/waits for stop and start of video stream" -> "display new video stream".
• (Item 12) Add HMI as an impacted platform.

[theresalech]

@dboltovskyi please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and enter issues in respective repositories for implementation. Thanks!

[theresalech]

The author has updated the proposal to include the requested revisions, and implementation issues have been entered:

• Core
• SDL HMI
• Generic HMI
• iOS
• Java Suite

[theresalech]

Added impacted platforms, based on accepted proposal revisions: https://github.com/smartdevicelink/sdl_evolution/issues/1059.