| Message ID | 20181023082513.30099-1-inigohuguet@fanamoel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] picotts: new package | expand |
Hello, Thanks for this contribution, seems like a useful package to have! See my review below. On Tue, 23 Oct 2018 10:25:13 +0200, Iñigo Huguet wrote: > PicoTTS, from SVOX, is the speech synthesizer included in Android > AOSP. It's lighweight (actually it compiles really fast), very > suitable for embedded devices, it sounds good and quite natural, > and have good support of all this languages: English, Spanish, this languages -> these languages > German, French and Italian. > > Languages support is probably the main improvements over the existing Languages support -> Language support > speech synthesizer packages, such as flite. > > Tested in desktop PC with Ubuntu and embedded device with Allwinner > A20 processor (ARMv7). > > The program only can output the synthesized sound to a wav file, and > you have to specify the language as an argument if it's not en_US. > I've added to the package to scripts that are installed to /usr/bin Don't use personal sentences such as "I've added". Something like "Two helper scripts have been added..." > to help with that: > - picotts: create the output wav file in /tmp, play it with aplay > and delete it > - picotts-lc: detect the current system language and call picotts > script However, I am not convinced that we want those helper scripts. They seem to be pretty use case specific, and it's not really the point of Buildroot to carry this type of script IMO. If you think they are generally useful, then talk with upstream to get them merged. However, I want to point out that this commit log is nice, and provide interesting details to understand the usefulness of this package. Thanks! > Signed-off-by: Iñigo Huguet <inigohuguet@fanamoel.com> > --- > package/Config.in | 1 + > package/picotts/Config.in | 8 ++++++ > package/picotts/picotts | 40 ++++++++++++++++++++++++++ > package/picotts/picotts-lc | 54 ++++++++++++++++++++++++++++++++++++ > package/picotts/picotts.hash | 1 + > package/picotts/picotts.mk | 34 +++++++++++++++++++++++ Please add an entry in the DEVELOPERS file for this new package. > diff --git a/package/picotts/Config.in b/package/picotts/Config.in > new file mode 100755 > index 0000000000..7ee5505fa0 > --- /dev/null > +++ b/package/picotts/Config.in > @@ -0,0 +1,8 @@ > +config BR2_PACKAGE_PICOTTS > + bool "picotts" > + select BR2_PACKAGE_POPT > + help > + PicoTTS: text-to-speech voice synthesizer from Android AOSP. > + It is lightweight and sounds quite good and natural. > + PicoTTS supports languages: English (British and American), PicoTTS supports the following languages: > + Spanish, German, French and Italian. Please add a blank line, and then the upstream URL of the project. > +picotts $lang_pico "$@" > diff --git a/package/picotts/picotts.hash b/package/picotts/picotts.hash > new file mode 100644 > index 0000000000..967544767c > --- /dev/null > +++ b/package/picotts/picotts.hash > @@ -0,0 +1 @@ > +sha256 1cc989efd85ea90ca228bbb694c4b842a909a232e2c8b9a307d241b3ddec246c picotts-2f86050dc5da9ab68fc61510b594d8e6975c4d2d.tar.gz > diff --git a/package/picotts/picotts.mk b/package/picotts/picotts.mk > new file mode 100755 > index 0000000000..02ccb4d13f > --- /dev/null > +++ b/package/picotts/picotts.mk > @@ -0,0 +1,34 @@ > +################################################################################ > +# > +# picotts > +# > +################################################################################ > + > +# pkg info Please don't add this type of comment, we don't have them anywhere in other Buildroot packages. > +PICOTTS_VERSION = 2f86050dc5da9ab68fc61510b594d8e6975c4d2d > +PICOTTS_SITE = $(call github,naggety,picotts,$(PICOTTS_VERSION)) > +PICOTTS_LICENSE = Apache-2.0 Sad that upstream doesn't include a license file :-/ > +# dependencies > +PICOTTS_DEPENDENCIES = popt > + > +# extract > +define PICOTTS_EXTRACT_CMDS > + tar xf $(PICOTTS_DL_DIR)/picotts-$(PICOTTS_VERSION).tar.gz -C $(@D) picotts-$(PICOTTS_VERSION)/pico --strip-components=2 > +endef Instead of this, could you try PICOTTS_SUBDIR = pico This tells the autotools-package infrastructure that all the autoconf/automake stuff is in the pico/ sub-directory. > +# generate build files (autotools) > +PICOTTS_PRE_CONFIGURE_HOOKS += PICOTTS_EXEC_AUTOGEN > +define PICOTTS_EXEC_AUTOGEN > + cd $(@D) && ./autogen.sh > +endef This won't work correctly, because you're not adding host-autoconf, host-automake and host-libtool in dependencies. Could you use PICOTTS_AUTORECONF = YES instead? If it doesn't work, could you give more details at what doesn't work? > +# install additional files > +PICOTTS_POST_INSTALL_TARGET_HOOKS += PICOTTS_INSTALL_ADDITIONAL_FILES > +define PICOTTS_INSTALL_ADDITIONAL_FILES > + $(INSTALL) -D -m 0755 $(TOPDIR)/$(PICOTTS_PKGDIR)/picotts $(TARGET_DIR)/usr/bin/picotts > + $(INSTALL) -D -m 0755 $(TOPDIR)/$(PICOTTS_PKGDIR)/picotts-lc $(TARGET_DIR)/usr/bin/picotts-lc $(TOPDIR)/$(PICOTTS_PKGDIR)/ can be simplified as $(PICOTTS_PKGDIR)/: all Buildoot make code is exactly from the Buildroot source code top-level directory. Could you rework the package to take into account those comments, and send an updated version ? Thanks a lot! Thomas
Hi, some comments below. El 02/11/18 a las 22:11, Thomas Petazzoni escribió: >> to help with that: >> - picotts: create the output wav file in /tmp, play it with aplay >> and delete it >> - picotts-lc: detect the current system language and call picotts >> script > However, I am not convinced that we want those helper scripts. They > seem to be pretty use case specific, and it's not really the point of > Buildroot to carry this type of script IMO. If you think they are > generally useful, then talk with upstream to get them merged. The problem is that there is no upstream anywhere. This program source was "extracted" from Android source, but there is not any "official" team maintaining it. Actually, I got the source code from Ubuntu's package and uploaded it to my own github. About the scripts being use case specific, I think they're not. The program only can create a .wav file, so if you want to use it to make "on the fly" speech, you need to create a temporary wav, play it, and delete it. This is in fact what the helper scripts do. I you think they must be removed I will do it, please tell me exactly what to do. Or, since the repository from which the package download the program is mine, I can include them to my repo directly. > Please add a blank line, and then the upstream URL of the project. Same problem, there is not any upstream URL. >> +# generate build files (autotools) >> +PICOTTS_PRE_CONFIGURE_HOOKS += PICOTTS_EXEC_AUTOGEN >> +define PICOTTS_EXEC_AUTOGEN >> + cd $(@D) && ./autogen.sh >> +endef > This won't work correctly, because you're not adding host-autoconf, > host-automake and host-libtool in dependencies. > > Could you use PICOTTS_AUTORECONF = YES instead? If it doesn't work, > could you give more details at what doesn't work? It will work if I just add those packages as dependencies? I will also try with autoreconf, I didn't do it because I don't know enough about how autotools works to know if there is any important difference or not. > Could you rework the package to take into account those comments, and > send an updated version ? > > Thanks a lot! > > Thomas Of course, I can. Regards Iñigo
Hello, On Mon, 5 Nov 2018 08:52:02 +0100, Iñigo Huguet wrote: > The problem is that there is no upstream anywhere. This program source > was "extracted" from Android source, but there is not any "official" > team maintaining it. Actually, I got the source code from Ubuntu's > package and uploaded it to my own github. Meh, Android is really wonderful open-source :-) > About the scripts being use case specific, I think they're not. The > program only can create a .wav file, so if you want to use it to make > "on the fly" speech, you need to create a temporary wav, play it, and > delete it. This is in fact what the helper scripts do. > > I you think they must be removed I will do it, please tell me exactly > what to do. Or, since the repository from which the package download the > program is mine, I can include them to my repo directly. Yes, it would be nicer if you included them directly in your repo. An example of something that was not completely correct is that your script uses aplay, but the package does not select BR2_PACAKAGE_ALSA_UTILS. But I believe it should not: there are possible use cases for this text-to-speech engine that don't involve playing the sound on the local device. > > Please add a blank line, and then the upstream URL of the project. > > Same problem, there is not any upstream URL. Just put the Github URL then :) > It will work if I just add those packages as dependencies? Yes, but we very much prefer to use <pkg>_AUTORECONF = YES when possible. Thanks! Thomas
diff --git a/package/Config.in b/package/Config.in index 6d2a73ff1b..97aef82b0b 100644 --- a/package/Config.in +++ b/package/Config.in @@ -44,6 +44,7 @@ menu "Audio and video applications" source "package/omxplayer/Config.in" source "package/on2-8170-libs/Config.in" source "package/opus-tools/Config.in" + source "package/picotts/Config.in" source "package/pulseaudio/Config.in" source "package/sox/Config.in" source "package/squeezelite/Config.in" diff --git a/package/picotts/Config.in b/package/picotts/Config.in new file mode 100755 index 0000000000..7ee5505fa0 --- /dev/null +++ b/package/picotts/Config.in @@ -0,0 +1,8 @@ +config BR2_PACKAGE_PICOTTS + bool "picotts" + select BR2_PACKAGE_POPT + help + PicoTTS: text-to-speech voice synthesizer from Android AOSP. + It is lightweight and sounds quite good and natural. + PicoTTS supports languages: English (British and American), + Spanish, German, French and Italian. diff --git a/package/picotts/picotts b/package/picotts/picotts new file mode 100755 index 0000000000..331aa97080 --- /dev/null +++ b/package/picotts/picotts @@ -0,0 +1,40 @@ +#!/bin/sh + +# help msg +if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then + echo "Usage: picotts <lang> <msg>" + echo "Langs: en-US, en-GB, es-ES, de-DE, fr-FR, it-IT" + exit 0 +fi + +# errors +if [ $# -lt 2 ]; then + echo "picotts: not enough arguments were given" >&2 + exit 1 +fi + +case "$1" in + en-US|en-GB|es-ES|de-DE|fr-FR|it-IT) + ;; + *) + echo "picotts: unsupported language: $1" >&2 + exit 1 + ;; +esac + +# create temp file +tmpfile=$(mktemp -tp /tmp picoSndXXXXXX) +tmpfilewav="$tmpfile.wav" +mv "$tmpfile" "$tmpfilewav" + +# extract language argument +lang_pico=$1 +shift + +# play message +pico2wave -l $lang_pico -w "$tmpfilewav" "$*" +aplay -q "$tmpfilewav" + +# delete temp file +rm "$tmpfilewav" + diff --git a/package/picotts/picotts-lc b/package/picotts/picotts-lc new file mode 100755 index 0000000000..a926f546ad --- /dev/null +++ b/package/picotts/picotts-lc @@ -0,0 +1,54 @@ +#!/bin/sh + +# at least 1 arg (the message to play) +if [ $# -eq 0 ]; then + echo "pitotts-lc: no arguments were given" >&2 + exit 1 +fi + +# help message +if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then + echo "Usage: picotts-lc <msg>" + echo "Language will be automatically detected from system locale" + echo "Supported languages: English, Spanish, German, French and Italian" + exit 0 +fi + +# get language configured in locale environment variable +if [ "$LC_ALL" != "" ]; then + lang=$LC_ALL +elif [ "$LC_MESSAGES" != "" ]; then + lang=$LC_MESSAGES +elif [ "$LANG" != "" ]; then + lang=$LANG +else + lang="C" +fi + +# check if language is supported by picoTTS +case "$lang" in + en_US*) + lang_pico="en-US" + ;; + en*) + lang_pico="en-GB" + ;; + es*) + lang_pico="es-ES" + ;; + de*) + lang_pico="de-DE" + ;; + fr*) + lang_pico="fr-FR" + ;; + it*) + lang_pico="it-IT" + ;; + *) + echo "picotts-lc: unsupported language: $lang" >&2 + exit 1 + ;; +esac + +picotts $lang_pico "$@" diff --git a/package/picotts/picotts.hash b/package/picotts/picotts.hash new file mode 100644 index 0000000000..967544767c --- /dev/null +++ b/package/picotts/picotts.hash @@ -0,0 +1 @@ +sha256 1cc989efd85ea90ca228bbb694c4b842a909a232e2c8b9a307d241b3ddec246c picotts-2f86050dc5da9ab68fc61510b594d8e6975c4d2d.tar.gz diff --git a/package/picotts/picotts.mk b/package/picotts/picotts.mk new file mode 100755 index 0000000000..02ccb4d13f --- /dev/null +++ b/package/picotts/picotts.mk @@ -0,0 +1,34 @@ +################################################################################ +# +# picotts +# +################################################################################ + +# pkg info +PICOTTS_VERSION = 2f86050dc5da9ab68fc61510b594d8e6975c4d2d +PICOTTS_SITE = $(call github,naggety,picotts,$(PICOTTS_VERSION)) +PICOTTS_LICENSE = Apache-2.0 + +# dependencies +PICOTTS_DEPENDENCIES = popt + +# extract +define PICOTTS_EXTRACT_CMDS + tar xf $(PICOTTS_DL_DIR)/picotts-$(PICOTTS_VERSION).tar.gz -C $(@D) picotts-$(PICOTTS_VERSION)/pico --strip-components=2 +endef + +# generate build files (autotools) +PICOTTS_PRE_CONFIGURE_HOOKS += PICOTTS_EXEC_AUTOGEN +define PICOTTS_EXEC_AUTOGEN + cd $(@D) && ./autogen.sh +endef + +# install additional files +PICOTTS_POST_INSTALL_TARGET_HOOKS += PICOTTS_INSTALL_ADDITIONAL_FILES +define PICOTTS_INSTALL_ADDITIONAL_FILES + $(INSTALL) -D -m 0755 $(TOPDIR)/$(PICOTTS_PKGDIR)/picotts $(TARGET_DIR)/usr/bin/picotts + $(INSTALL) -D -m 0755 $(TOPDIR)/$(PICOTTS_PKGDIR)/picotts-lc $(TARGET_DIR)/usr/bin/picotts-lc +endef + +# build +$(eval $(autotools-package))
PicoTTS, from SVOX, is the speech synthesizer included in Android AOSP. It's lighweight (actually it compiles really fast), very suitable for embedded devices, it sounds good and quite natural, and have good support of all this languages: English, Spanish, German, French and Italian. Languages support is probably the main improvements over the existing speech synthesizer packages, such as flite. Tested in desktop PC with Ubuntu and embedded device with Allwinner A20 processor (ARMv7). The program only can output the synthesized sound to a wav file, and you have to specify the language as an argument if it's not en_US. I've added to the package to scripts that are installed to /usr/bin to help with that: - picotts: create the output wav file in /tmp, play it with aplay and delete it - picotts-lc: detect the current system language and call picotts script Signed-off-by: Iñigo Huguet <inigohuguet@fanamoel.com> --- package/Config.in | 1 + package/picotts/Config.in | 8 ++++++ package/picotts/picotts | 40 ++++++++++++++++++++++++++ package/picotts/picotts-lc | 54 ++++++++++++++++++++++++++++++++++++ package/picotts/picotts.hash | 1 + package/picotts/picotts.mk | 34 +++++++++++++++++++++++ 6 files changed, 138 insertions(+) create mode 100755 package/picotts/Config.in create mode 100755 package/picotts/picotts create mode 100755 package/picotts/picotts-lc create mode 100644 package/picotts/picotts.hash create mode 100755 package/picotts/picotts.mk