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

Oboe: Add device specific Quirks for Samsung #734

Merged
merged 5 commits into from
Feb 11, 2020
Merged

Oboe: Add device specific Quirks for Samsung #734

merged 5 commits into from
Feb 11, 2020

Conversation

philburk
Copy link
Collaborator

@philburk philburk commented Dec 19, 2019

Use a minimum of 2 bursts for MMAP on Exynos devices.
Add OboeGlobals::setWorkaroundEnabled() for testing
so we can repro AAudio bugs using Oboe.

Note Oboe API change. OboeGlobals added.

Test: run OboeTester, look at MMAP output buffer size min
Test: Disable and enable workarounds from MainActivity.

Use a minimum of 2 bursts for MMAP on Exynos devices.
Add OboeGlobals::setWorkaroundEnabled() for testing
so we can repro AAudio bugs using Oboe.

Test: run OboeTester, look at MMAP output buffer size min
Test: Disable and enable workarounds from MainActivity.
Copy link
Collaborator Author

@philburk philburk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note API change: added static OboeGlobals class.

include/oboe/Definitions.h Outdated Show resolved Hide resolved
include/oboe/Definitions.h Outdated Show resolved Hide resolved
@dturner dturner added the requires device Requires a specific Android phone model or other specific hardware to test label Dec 19, 2019
Phil Burk added 2 commits December 20, 2019 10:59
Bump OboeTester version number to 1.5.22
Bump Oboe version number to 1.3.1
@philburk
Copy link
Collaborator Author

This is related to - Glitches on S8,S9 #564
and - Limit setBufferSize to reduce glitches #654

@dturner
Copy link
Collaborator

dturner commented Jan 7, 2020

Tested on Samsung Galaxy S9 SM-G960F Build number PPR1.180610.011.G960FXXU2CSA2 (Exynos 9810)

Setup:

  • Build and run OboeTester (from this quirksize branch)
  • Connect headphones using 3.5mm headphone socket

Steps to test (workarounds OFF):

  • Tap TEST OUTPUT
  • Tap OPEN then START

Outcome:

  • Sine wave with lots of audible artifacts (crackles, pops etc) is heard
  • Buffer size listed as 96 frames

Steps to test (workarounds ON)

  • Tap back
  • Check "enable Oboe workarounds"
  • Tap TEST OUTPUT
  • Tap OPEN then START

Outcome:

  • Less audible artifacts heard, but still very noticeable. I'd estimate at one artifact per second. These do not show up as xruns.
  • Buffer size is set to 192

Summary: Increasing the buffer size definitely reduces the number of glitches, but doesn't solve the problem entirely.

@@ -495,6 +495,25 @@ namespace oboe {
int64_t timestamp; // in nanoseconds
};

class OboeGlobals {
Copy link
Collaborator

@dturner dturner Jan 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with the workarounds being set here. I wonder if the default stream values should be moved into this class as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add getters/setters for the default stream values e.g. setOpenSLESDefaultSampleRate etc
Add comment saying that direct access to DefaultStreamValues is deprecated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other thing. It might be better to name this Globals rather than OboeGlobals since this is already in the oboe namespace.

@dturner
Copy link
Collaborator

dturner commented Jan 8, 2020

@dturner Take a look at the S9 properties to correctly identify it, then choose a buffer size which eliminates glitches.

@dturner dturner self-assigned this Jan 8, 2020
@dturner
Copy link
Collaborator

dturner commented Jan 8, 2020

Do adb shell getprop then grep for vendor, should get model no etc. Send to Phil.

@Nordhal
Copy link

Nordhal commented Feb 7, 2020

Will it be included on the 1.3.3 then ? The version was bumped from the quirksize branch but not merged even at oboe 1.3.2

@philburk philburk merged commit b9110f9 into master Feb 11, 2020
@philburk philburk deleted the quirksize branch February 11, 2020 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requires device Requires a specific Android phone model or other specific hardware to test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants