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

Fix Bitrate Update for Video Streaming #1413

Merged

Conversation

t-yoshii
Copy link

Fixes #1392

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

I tested with our internal devboard.

Summary

reflect bitrate value returned from HMI in video encoder settings, except the case that app specified custom bitrate in encoder settings.

Changelog

Enhancements
  • reflect bitrate value returned from HMI in video encoder settings, except the case that app specified custom bitrate in encoder settings.

CLA

@t-yoshii t-yoshii changed the base branch from master to develop September 26, 2019 09:44
@t-yoshii
Copy link
Author

Hello @NicoleYarroch @joeljfischer ,

#1392 is resolved by #1393, however, we have reconsidered how to fix #1392

Fix #1393 has 2 problems.

  1. By default, customVideoSettings is overwritten by HMI capabilities. This breaks current SDL app behavior which uses customeVideoSettings.
  2. The flag allowOverrideEncoderSettings is a little bit confusing.

So, we applied a new fix with following rules.
a. customVideoEncoderSettings is always applied. i.e. customVideoEncoderSettings > HMI capabilities > defaultVideoEncoderSettings.
b. Do not introduce additional flag (such as allowOverrideEncoderSettings) for simplicity.

Since customVideoSettings is the value that is explicitly set by the application, it should always have the highest priority (than HMI's value).

Could you please kindly review this again? I am sorry for requesting review twice. Thank you.

@joeljfischer joeljfischer changed the title Fix/reflect bitrate refix Fix Bitrate Update for Video Streaming Sep 30, 2019
@joeljfischer joeljfischer added the bug A defect in the library label Sep 30, 2019
@joeljfischer joeljfischer added this to In progress in v6.4 via automation Sep 30, 2019
@NicoleYarroch NicoleYarroch added this to the 6.4.0 milestone Oct 3, 2019
v6.4 automation moved this from In progress to Reviewer approved Oct 3, 2019
@joeljfischer joeljfischer merged commit 11e7c4b into smartdevicelink:develop Oct 3, 2019
v6.4 automation moved this from Reviewer approved to Done Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library
Projects
No open projects
v6.4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants