-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 the sleep logic for streaming as per sample rate #279
Conversation
Current coverage is 47.22% (diff: 85.71%)@@ master #279 diff @@
==========================================
Files 78 78
Lines 2267 2270 +3
Methods 0 0
Messages 0 0
Branches 154 154
==========================================
+ Hits 1018 1072 +54
+ Misses 1216 1167 -49
+ Partials 33 31 -2
|
// For 16000 Hz sample rate, sleep 100 milliseconds. | ||
Thread.sleep(samplingRate / 160); | ||
// Sleep 100ms | ||
Thread.sleep(100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approximating realtime is dependent on the samplingRate
, so you can't hard-code 100ms unless you're also hard-coding the samplingRate
.
The calculation here should instead be dependent on samplingRate
, buffer size, and encoding. Since encoding is hard-coded to 2 bytes per sample, you should sleep for:
int BYTES_PER_SAMPLE = 2; // because LINEAR16 - ie 16 bits per sample, at 8 bits per byte
int BYTES_PER_BUFFER = 3200;
int SAMPLES_PER_BUFFER = BYTES_PER_BUFFER / BYTES_PER_SAMPLE; // 1600
int samplesPerMillisecond = samplingRate / 1000;
int millisecondsPerBuffer = SAMPLES_PER_BUFFER / samplesPerMillisecond;
Thread.sleep(millisecondsPerBuffer);
which for 16000 Hz is 1600 / 16 = 100
(as expected), and for 32000 Hz is 1600 / 32 = 50
(which is faster than the original 32000 / 160 = 200
). I suspect there was just an error in the original calculation. (feel free to check my math as well - but at least it gives a shorter sleep duration, which is what you'd expect if you increase the sample rate while keeping the buffer size the same)
@jerjou PTAL |
Cool. |
@jerjou PTAL |
@@ -52,11 +52,13 @@ | |||
*/ | |||
@RunWith(JUnit4.class) | |||
public class StreamingRecognizeClientTest { | |||
private TestAppender appender; | |||
private Writer writer; | |||
private WriterAppender appender; | |||
|
|||
@Before |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a @BeforeClass
, since it's adding an appender (rather than replacing it)? ie if this ones once per test, won't new appenders get added as many times as there are tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to do it for each test so that StringWriter is new. There is an After which I now think should be AfterClass where all added appenders will be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. No - having it in an After makes sense. Sorry - didn't see the After.
Travis was unhappy because the api wasn't enabled in the test project. I enabled it and kicked off the build again (I think?). Thanks for adding the tests! Didn't realize there weren't already tests -_-; Anyway - looks good from my end - just a couple minor comments. Can you get someone from the java team to sign off on it? |
Oh - would you mind updating the title to the PR? |
Minor comment, consider it, change if you wish, then LGTM |
LGTM - once travis passes. |
…1.0 (#279) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [com.google.cloud:libraries-bom](https://togithub.com/GoogleCloudPlatform/cloud-opensource-java) | minor | `12.0.0` -> `12.1.0` | --- ### Renovate configuration :date: **Schedule**: At any time (no schedule defined). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. :no_bell: **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-datalabeling).
…1.0 (#279) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [com.google.cloud:libraries-bom](https://togithub.com/GoogleCloudPlatform/cloud-opensource-java) | minor | `12.0.0` -> `12.1.0` | --- ### Renovate configuration :date: **Schedule**: At any time (no schedule defined). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. :no_bell: **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-datalabeling).
🤖 I have created a release \*beep\* \*boop\* --- ### [0.120.5](https://www.github.com/googleapis/java-errorreporting/compare/v0.120.4...v0.120.5) (2020-10-21) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([#277](https://www.github.com/googleapis/java-errorreporting/issues/277)) ([372fca0](https://www.github.com/googleapis/java-errorreporting/commit/372fca01cbac38951689f533b8d76d2a0bdc74e9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
🤖 I have created a release \*beep\* \*boop\* --- ### [0.120.5](https://www.github.com/googleapis/java-errorreporting/compare/v0.120.4...v0.120.5) (2020-10-21) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([#277](https://www.github.com/googleapis/java-errorreporting/issues/277)) ([372fca0](https://www.github.com/googleapis/java-errorreporting/commit/372fca01cbac38951689f533b8d76d2a0bdc74e9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
🤖 I have created a release \*beep\* \*boop\* --- ### [0.120.5](https://www.github.com/googleapis/java-errorreporting/compare/v0.120.4...v0.120.5) (2020-10-21) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([#277](https://www.github.com/googleapis/java-errorreporting/issues/277)) ([372fca0](https://www.github.com/googleapis/java-errorreporting/commit/372fca01cbac38951689f533b8d76d2a0bdc74e9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
…uration to v1.0.21 (#279) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [com.google.cloud.samples:shared-configuration](com/google/cloud/samples/shared-configuration) | patch | `1.0.18` -> `1.0.21` | --- ### Renovate configuration :date: **Schedule**: At any time (no schedule defined). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. :no_bell: **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-recommender).
…-private-ca to v2.1.1 (#279) [![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-security-private-ca](https://togithub.com/googleapis/java-) | `2.1.0` -> `2.1.1` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/compatibility-slim/2.1.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/confidence-slim/2.1.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-security-private-ca).
* samples: add create agent code sample * Lint fix * Fix failing test * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Rebased * Revised Code per comments * lint fix * Added Comment for timezone * Fixed lint * Used String.format * Added stdOut Var * removed parent variable * Update Tests * lint fix * removed parentPath * removed unreachable statemt * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Revised Code Per Comments * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* samples: add create agent code sample * Lint fix * Fix failing test * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Rebased * Revised Code per comments * lint fix * Added Comment for timezone * Fixed lint * Used String.format * Added stdOut Var * removed parent variable * Update Tests * lint fix * removed parentPath * removed unreachable statemt * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Revised Code Per Comments * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
…-private-ca to v2.1.1 (#279) [![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-security-private-ca](https://togithub.com/googleapis/java-) | `2.1.0` -> `2.1.1` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/compatibility-slim/2.1.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-security-private-ca/2.1.1/confidence-slim/2.1.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-security-private-ca).
@jerjou @tswast Please review. Changed the sleep time to absolute 100ms because for 32KKHz the Speech API was complaining about
message: "Audio data is being streamed too slow. Please stream audio data approximately at real time."