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.

Sign up for GitHub

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

Jump to bottom

Review handling of DataCallbackResult::Stop #1230

Closed
philburk opened this issue Mar 22, 2021 · 2 comments · Fixed by #1351
Closed

Review handling of DataCallbackResult::Stop #1230

philburk opened this issue Mar 22, 2021 · 2 comments · Fixed by #1351
Assignees
@philburk
Labels
P1 high priority
Milestone
V1.6.1

Comments

@philburk
Copy link
Collaborator

Consider affect on thread joining before S.
Maybe add a blocker to prevent app callbacks.
Add some unit tests.
@philburk philburk self-assigned this Mar 22, 2021
@philburk philburk added the P1 high priority label Mar 22, 2021
@philburk philburk added this to the v1.5.2 milestone Mar 22, 2021
@philburk philburk modified the milestones: v1.5.2, v1.6 May 15, 2021
@philburk
Copy link
Collaborator Author

AAudio handles the STOP request here:

oboe/src/aaudio/AudioStreamAAudio.cpp

Line 369 in bfda790

if (getSdkVersion() <= __ANDROID_API_P__) {

Bugs related to AAUDIO_CALLBACK_RESULT_STOP

b/182954108 - can lead to a race and hang in Legacy threads. Fixed in S.
b/121158739 - can cause a Legacy stream to not restart. Fixed in Q

b/171296283 - it can leave the MMAP stream in a bad state where the thread does not get joined properly.
b/120845500 - log spam that can occur in P

Conclusion: AAUDIO_CALLBACK_RESULT_STOP should not be used before S.
Oboe should block callbacks and use a separate thread to stop the stream.

philburk added a commit that referenced this issue Jul 17, 2021
@philburk
oboe: improve DataCallbackResult::Stop handling
44046c7 
Use another thread to stop the stream for R and earlier.

Fixes #1230.
Test: use OboeTester "callback return STOP" checkbox
@philburk philburk mentioned this issue Jul 17, 2021
Merged
philburk added a commit that referenced this issue Jul 21, 2021
@philburk
oboe: improve DataCallbackResult::Stop handling (#1351)
8ff99cc 
Use another thread to stop the stream for R and earlier.

Test: use OboeTester "callback return STOP" checkbox

Added a unit test, testReturnStop.cpp
run_tests.sh now does not show logcat if app build fails.

Fixes #1230.
@Fanzijian1996
Copy link

As an Android application developer, I'm reading up on AudioStreamAAudio.cpp, I have a questions.

  1. why should we use use a separate thread to stop the stream? if we want to stop, we can use requeststop outside.
    Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@philburk philburk

Labels
P1 high priority
Projects
None yet
Milestone
V1.6.1
Development

Successfully merging a pull request may close this issue.

oboe: improve DataCallbackResult::Stop handling google/oboe
2 participants
@philburk @Fanzijian1996