-
Notifications
You must be signed in to change notification settings - Fork 122
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 0297 - Add function to transmit audio data of AudioStream in time division #988
Comments
|
@joeljfischer -san
|
2. Can I ask which existing head units? |
@joeljfischer -san |
1. Because the "sample code" provided here is so extensive I would ask that this proposal be deferred until such a time that the iOS and Java_Suite teams are capable of reviewing it in more detail. 2. I think that before this could be accepted, it would have to be tested on all existing head units for potential regressions because it is such an aggressive change to video streaming. Could you provide an example fork or otherwise accessible code of the sdl_ios and sdl_java_suite repositories with this example code integrated and an example app that plays audio in this manner in order for OEMs to test this change with their head units? |
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. |
@joeljfischer -san |
I think the best way to share this would be to create a fork of the SDL_iOS and SDL_Java_Suite repositories, then to make the required changes for this proposal in the library so that everyone can verify the changes and build it into a sample app to test. Please use the latest master branch of the sdl_ios and sdl_java_suite libraries. You can then share a link to the fork. Because this proposal involves changing how data is sent and when, I believe it would be best for all SDLC members with hardware to then test the changes and ensure that their hardware properly handles it before voting to accept this proposal. |
The Steering Committee voted to defer this proposal. The author will follow the instructions provided by the Project Maintainer in this comment to build sample applications implementing the proposed feature. SDLC Members will then test the sample applications on their own hardware to ensure it will work with their existing head units. Once that testing is completed, this proposal can be brought back into review for Steering Committee vote. |
Steering Committee Representatives, please find information from the author regarding their Android sample application and request for assistance testing below. More information to follow regarding the iOS sample application.
|
Steering Committee Representatives, below is the information from the author regarding testing the iOS sample application. As noted in the original Steering Committee decision for this proposal, SDLC Members should now test the sample applications on their own hardware to ensure it will work with their existing head units. Once that testing is completed, this proposal can be brought back into review for Steering Committee vote.
|
Sample App feedback provided by Xevo team (@shiniwat):
|
The author has updated both the iOS and Android sample applications to address the feedback received: iOS: https://github.com/zx5656/sdl_video_streaming_iOS_sample
Android: https://github.com/zx5656/sdl_video_streaming_android_sample
|
Updated Sample App feedback provided by Xevo team:
|
Author has provided updated iOS Sample App for testing: https://github.com/zx5656/sdl_video_streaming_iOS_sample |
Closing as inactive. The issue will remain in a |
I think we (Xevo team) have provided the feedback already, but here's the summary of our test results:
Just a minor concern is that sdl_java_suite library in the sample app is based on 4.11.1, while the latest release is 4.12.0. Likewise, iOS library in the same app is based on 6.6.0, while the latest release is 6.7.0. However it won't be a big deal for sdl_evolution process. |
@theresalech -san |
@Shohei-Kawano, we understand your concern! I will bring the following to the Steering Committee during our next meeting (2020-09-22), so that we can move to bring this proposal back into review:
|
Please find the summary below of the Ford team's test results: iOS sample app - video and audio streaming are working well, the audio playback is fluent. We share the minor concern that Xevo has raised regarding testing with the latest iOS and JavaSuite libraries, however we also agree that this should not be a blocking issue for an evolution proposal. |
As SDLC members have tested the sample apps provided by the author, and provided feedback on this review issue, they have now voted to bring this proposal back into review. This proposal will remain in review until 2020-10-06. |
I am unsure if I could evaluate the android implementation but the iOS one is looking good so far. Note: There is a big overlap with another proposal I am working on #1013 which is deprecating the audio streaming manager and replacing it with another manager supporting reliable input and output streaming. It's likely that your proposal will get accepted before mine which means I will need to make sure your concept is applied to the new manager as well if my proposal deprecates the current audio manager. 3. The proposal's iOS implementation contains 4. In my proposal I am referring to an issue I found with Ford IVIs which occurs if the system's input buffer empties and new data is received at the same time. There's a race condition where the system detects the input stream being empty, it'll shut down the prompt playback which takes a few milliseconds but if the app is sending audio at this short time slot it can happen that this audio data won't be played until the next audio data is send. Let me try to explain with the below diagrams. The below diagram shows the happy path with the current behavior where new data is sent if the audio file (or in your case the chunk) is being played that results in a small time slot where the buffer is empty. In the second diagram below I want to demonstrate the issue. The system got caught in the race condition, shutting down the audio playback but at the same time new data is being send. The system won't play the audio because it's not checking it's status again. After the third data transfer, the system recovers and ramps up the playback dequeuing now all data. In the third diagram you see the attempt of my proposal preventing the buffer from being empty if the app has data in the pipeline. I was able to reproduce the issue with the current library implementation. I assume your proposal won't change the behavior but cause more cases when the buffer becomes empty. I'm afraid the issue could occur more often. Long story short: I just want to make you aware of this proposed change. My proposal will have impact to how the manager behaves to fill the system's audio buffer and it may have impact to your implementation. However your motivation and your concept won't be affected. |
@kshala-ford -san, Thank you for your consideration and support. |
In preparation for tonight's Steering Committee meeting, I'd like to propose accepting this proposal with revisions. It's our suggestion that the author revise to include references to the sample apps, and state that the code included in the proposal is sample code. |
The Steering Committee voted to accept this proposal with revisions. The author will revise the proposal to include references to the sample apps provided in the comments, and state that the code included in the proposal is sample code. |
@Akihiro-Miyazaki, 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! |
Proposal has been updated and implementation issues have been entered: |
Hello SDL community,
The review of "SDL 0297 - Add function to transmit audio data of AudioStream in time division" begins now and runs through October 6, 2020. The original review of this proposal took place April 1 - 14, 2020. The proposal is available here:
https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0297-Add-function-to-transmit-audio-data.md
Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:
#988
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:
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
The text was updated successfully, but these errors were encountered: