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

Change sample rate of pico2wave #293

Merged
merged 13 commits into from
Jun 22, 2017
Merged

Change sample rate of pico2wave #293

merged 13 commits into from
Jun 22, 2017

Conversation

corus87
Copy link
Contributor

@corus87 corus87 commented Jun 21, 2017

I wanted to give pico a try but I had a bad sound caused by my usb sound card which does not support 16 kHz.
I found some reference with other usb devices like the Jabra speaker, but the mention solutions with the .asoundrc didn't work for me, only changing the sample rate with sox fixed my problem.
Maybe someone else will face the same issue, with a cheap amazon usb sound cards like mine or even with a Jabra speaker.

@corus87 corus87 changed the title Dev Change sample rate of pico2wave Jun 21, 2017
@LaMonF LaMonF added this to the v0.4.5 milestone Jun 21, 2017
LaMonF
LaMonF requested changes Jun 21, 2017
View reviewed changes
Copy link
Member

@LaMonF LaMonF left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Travis CI tests are failing because the "sox" dependencies are missing.
Make sure dependencies files are updated :

@corus87
Copy link
Contributor Author

corus87 commented Jun 21, 2017 •
edited
Loading

Sry, I was not sure where to put the requirements.

@coveralls
Copy link

coveralls commented Jun 21, 2017 •
edited
Loading

Coverage Status

Changes Unknown when pulling 6638e6f on corus87:dev into ** on kalliope-project:dev**.

@coveralls
Copy link

coveralls commented Jun 21, 2017 •
edited
Loading

Coverage Status

Changes Unknown when pulling 6638e6f on corus87:dev into ** on kalliope-project:dev**.

@coveralls
Copy link

coveralls commented Jun 21, 2017 •
edited
Loading

Coverage Status

Changes Unknown when pulling 6638e6f on corus87:dev into ** on kalliope-project:dev**.

@LaMonF
Copy link
Member

LaMonF commented Jun 21, 2017

Thanks for your PR !
Instead of using change_rate parameter as a boolean, I think it could be more flexible to let the user define the rate.

eg :

  • name the parameter samplerate and let the user define an int value.
  • Parameter default value could be null.
  • So if the value is not null, apply the sox build.

@corus87 what do you think ?

@corus87
Copy link
Contributor Author

corus87 commented Jun 21, 2017 •
edited
Loading

This sounds very good! Never though about a flexible sample rate, I have not even tried a lower sample rate with my sound card, maybe a lower one will works as well.
I will take care of it.
edit:
@LaMonF I have made the changes, what do you think?

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e2fe762 on corus87:dev into ** on kalliope-project:dev**.

@coveralls
Copy link

coveralls commented Jun 21, 2017 •
edited
Loading

Coverage Status

Changes Unknown when pulling e2fe762 on corus87:dev into ** on kalliope-project:dev**.

@coveralls
Copy link

coveralls commented Jun 21, 2017 •
edited
Loading

Coverage Status

Changes Unknown when pulling e2fe762 on corus87:dev into ** on kalliope-project:dev**.

@Sispheor
Copy link
Member

@monf. Do a squash merge et not a merge please at the end of the process.

LaMonF
LaMonF reviewed Jun 21, 2017
View reviewed changes
kalliope/tts/pico2wave/README.md Outdated
@@ -6,6 +6,7 @@ This TTS is based on the SVOX picoTTS engine
|------------|----------|---------|--------------|-------------------------------------------------|
| language | YES | | 6 languages | List of supported languages in the Note section |
| cache | No | TRUE | True / False | True if you want to use the cache with this TTS |
| samplerate| No|None |int| Pico2wave creates 16 khz files but not all USB devices support this. Set a value to convert to a specific samplerate. For Example: 44100 |
Copy link
Member

Choose a reason for hiding this comment

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

Please make a 'clean' table :) using white spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sry, I fixed it

@coveralls
Copy link

coveralls commented Jun 21, 2017 •
edited
Loading

Coverage Status

Changes Unknown when pulling 34f06f9 on corus87:dev into ** on kalliope-project:dev**.

@LaMonF LaMonF merged commit 6ac8fc2 into kalliope-project:dev Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LaMonF LaMonF LaMonF requested changes

Assignees
No one assigned
Labels
Community Module enhancement
Projects
None yet
Milestone
v0.4.5
Development

Successfully merging this pull request may close these issues.
4 participants
@corus87 @coveralls @LaMonF @Sispheor