Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
theresalech opened this issue Jul 29, 2020 · 7 comments

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "Revise SDL-0296 - Possibility to update video streaming capabilities during ignition cycle" begins now and runs through August 4, 2020.

This will be a review of proposed revisions to a previously accepted but not yet implemented proposal, SDL 0296.

The pull request outlining the revisions under review is available here:

#1055

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

#1059

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]

@joeljfischer
Copy link
Contributor

1. The Java versions appears to be missing some methods that are available on iOS:

// 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;

2. The Resolution Switching section has become a sub-section of JavaScript APIs, was that intentional?

@dboltovskyi
Copy link
Contributor

That's a mistake that needs to be corrected: JavaScript APIs section has to be after Resolution Switching

@dboltovskyi
Copy link
Contributor

It was not planned initially to have these methods. But in order to have better alignment with iOS changes we would propose to add the similar methods for Android:

public Boolean isImageResolutionInRange(VideoStreamingRange range, ImageResolution currentResolution)
public Boolean isAspectRatioInRange(VideoStreamingRange range, ImageResolution currentResolution)

@joeljfischer
Copy link
Contributor

👍

@smartdevicelink smartdevicelink locked as resolved and limited conversation to collaborators Aug 5, 2020
@theresalech theresalech changed the title [In Review] SDL 0296 Revisions - Possibility to update video streaming capabilities during ignition cycle [Accepted with Revisions] SDL 0296 Revisions - Possibility to update video streaming capabilities during ignition cycle Aug 5, 2020
@theresalech
Copy link
Contributor Author

The Steering Committee voted to accept this proposal with revisions. The author will move the JavaScript API section after Resolution Switching and also add the following methods to Android:

public Boolean isImageResolutionInRange(VideoStreamingRange range, ImageResolution currentResolution)
public Boolean isAspectRatioInRange(VideoStreamingRange range, ImageResolution currentResolution)

@theresalech
Copy link
Contributor Author

@dboltovskyi please advise when your PR has been updated to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and leave comments on the implementation issues to reference these updates. Thanks!

@theresalech
Copy link
Contributor Author

theresalech commented Aug 5, 2020

Author has updated proposal to reflect agreed upon revisions, and comments have been left on implementation issues:

Issues for projects now impacted by this feature:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants