Skip to content

Espeak support#14

Open
boyska wants to merge 4 commits into
LegolasTheElf:masterfrom
boyska:espeak
Open

Espeak support#14
boyska wants to merge 4 commits into
LegolasTheElf:masterfrom
boyska:espeak

Conversation

@boyska

@boyska boyska commented Aug 9, 2015

Copy link
Copy Markdown

I got banned by google, so I wrote this patch that seems to me of general interest.

With --espeak you can use espeak: the default is still google. You can, however, be more explicit with --google.

They only make sense if you use --audio.

I also did some minor refactoring of the audio-related code.

@peppeska

Copy link
Copy Markdown

Questa opzione direi che e' fondamentale! 😎

aleskandro pushed a commit that referenced this pull request Feb 5, 2017

@rfc1459 rfc1459 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Riassunto:

  • cambio non necessario dell'interprete che - tra l'altro - rompe la compatibilità con sistemi dove bash non è in /bin/bash
  • bashismi (ecco perché è stato cambiato l'interprete)
  • merge conflict

Come minimo ci vuole un rebase, ma direi che l'intera PR non va bene. (nota a margine: chi ha privilegi un pelino più alti dei miei potrebbe attaccare il repo a Travis CI così da eseguire shellcheck su ogni PR)

Comment thread mannaggia.sh Outdated
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, no, no.
E se non si fosse capito: no.

Non ha senso modificare l'interprete solo perché ci sono dei bashismi.

Comment thread mannaggia.sh Outdated

espeak_best_voice() {
v=$(espeak --voices=it | tail -n +2 | awk '{ print $4 }' | grep -v mbrola | head -n1)
if [[ -n "$v" ]]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bashismo

Comment thread mannaggia.sh Outdated
return
fi
v=$(espeak --voices=it | tail -n +2 | awk '{ print $4 }' | head -n1)
if [[ -n "$v" ]]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Altro bashismo

@boyska

boyska commented Feb 6, 2017

Copy link
Copy Markdown
Author

ti va di chiarire il bashismo? trovo sempre difficile riconoscere i bashismi perché l'unico modo per farlo è leggere l'intero elenco di bashismi e capire cosa non va (o non imparare bash ma solo shell posix, certo). Se mi dai una dritta al volo poi provo a fixare.

@rfc1459

rfc1459 commented Feb 6, 2017

Copy link
Copy Markdown
Contributor

[[ ]] è un bashismo (POSIX prevede solo [ ]), comunque ti conviene usare Shellcheck per verificare questo tipo di problemi.

@boyska

boyska commented Feb 6, 2017

Copy link
Copy Markdown
Author

ok grazie, credo di aver risolto. In realtà shellcheck continua a lamentarsi, ma secondo me si sbaglia lui. Dice che

echo "-v $v -k10 -g1 -p 30"

non va bene perché In POSIX sh, echo flags are undefined ma secondo me non c'entra nulla, io l'ho quotato, quello non è un flag ad echo, boh

@rfc1459

rfc1459 commented Feb 6, 2017

Copy link
Copy Markdown
Contributor

E' decisamente un falso positivo, prima della riga incriminata aggiungi # shellcheck disable=SC2039

@rfc1459 rfc1459 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Much much better :-)
Manca giusto un rebase sul master attuale.

@boyska

boyska commented Feb 6, 2017 via email

Copy link
Copy Markdown
Author

@rfc1459

rfc1459 commented Feb 6, 2017

Copy link
Copy Markdown
Contributor

Ma poi ispeech.org non funziona, lamentandosi (in modo spassosissimo) che "invalid license, please visit ispeech.org for a valid license".

Che belli i servizi che nel giro di un anno (data del merge di #15) da free diventano a pagamento...

Mi sa che l'unica è ranzare del tutto il supporto ai TTS esterni e usare solo espeak se disponibile sulla macchina.
@henryx, @LegolasTheElf, @aleskandro, @dariomas: commenti?

@boyska

boyska commented Feb 6, 2017 via email

Copy link
Copy Markdown
Author

@henryx

henryx commented Feb 6, 2017

Copy link
Copy Markdown
Collaborator

Probabilmente rimuovere il supporto ai TTS mi sembra l'unica soluzione, a meno che non si trovi un'alternativa valida (e.g. Zanata, Bing o Yandex)

@LegolasTheElf

LegolasTheElf commented Feb 6, 2017 via email

Copy link
Copy Markdown
Owner

@LegolasTheElf

LegolasTheElf commented Feb 6, 2017 via email

Copy link
Copy Markdown
Owner

@LegolasTheElf LegolasTheElf left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

i don't like anymore TTS services via internet because of the rapid change of their licensing and of their availability.
i propose to evaluate the idea of leaving TTS ONLY as a flag called on the command line, wiping it at all or use a local package adding it as a dependence

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants