smartdevicelink/sdl_evolution

[Accepted with Revisions] SDL 0332 - Additional Video Streaming Capabilities Validation

theresalech opened this issue · 8 comments

theresalech commented

Hello SDL community,

The review of "SDL 0332 - Additional Video Streaming Capabilities Validation" begins now and runs through May 18, 2021. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0332-additional-video-streaming-capabilities-validation.md

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

#1139

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
theresa@livio.io

joeljfischer commented

1. In "Validation of HMI Notification OnSystemCapabilityUpdated" you state:

No duplicate capabilities should be included in the set of the root capability and the additional capabilities.

I believe that you need to define how to determine what is a duplicate and what is not. Non-screen-size related parameters should not count toward duplicate types, such as preferredFPS, maxBitrate, supportedFormats, or hapticSpatialDataSupported. In other words, if two capabilities are the same except for preferredFPS, they should still be considered duplicates.

2. In "Validation of RPC OnAppCapabilityUpdated"

No root level capability should be included in the OnAppCapability notification. All supported resolutions should be included in the parameter additionalStreamingCapabilities.

I think this is a little vague. I think you mean that the OnAppCapabilityUpdated.appCapability.videoStreamingCapability should only include all of its data in .additionalVideoStreamingCapabilities, and not that the OnAppCapabilityUpdated should not include the root capability from the HMI's video streaming capabilities. I think providing an example response for the app capability might be helpful here.

  • 👍1
Jack-Byrne commented

@joeljfischer Could you please elaborate on this?

In other words, if two capabilities are the same except for preferredFPS, they should still be considered duplicates.

Why should preferredFPS and other non screen size capabilities not be counted as a unique to the capability?

  1. Thank you for the suggestion, I can update the proposal to be more specific in regards to the parameters I am referencing.
joeljfischer commented

1. My concern is for an HMI developer creating an identical screen size (combining all the relevant factors), but providing multiple preferredFPS, maxBitrate, etc. and those being considered by Core to not be duplicates. Each given screen size should provide one and only one preferredFPS, maxBitrate, etc.

Jack-Byrne commented
  1. Thank you. I would like to add this as a validation requirement to the proposal.

A unique screen size is determined by the following parameters: preferredResolution, diagonalScreenSize, pixelPerInch, and scale. Each given screen size should provide one and only one preferredFPS, maxBitrate, supportedFormats, and hapticSpatialDataSupported.

  • 👍1
jordynmackool commented

Summarized agreed upon items for revision as discussed in the comments above:

  1. Add the following validation requirement "A unique screen size is determined by the following parameters: preferredResolution, diagonalScreenSize, pixelPerInch, and scale. Each given screen size should provide one and only one preferredFPS, maxBitrate, supportedFormats, and hapticSpatialDataSupported.
  2. Add example response and parameter details for OnAppCapability.
theresalech commented

The Steering Committee voted to accept this proposal with the revisions summarized in this comment.

theresalech commented

@JackLivio 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 impacted repositories for implementation. Thanks!

theresalech commented

Author has updated proposal, and implementation issues have been entered: