Bug 209311 - Review Request: espeak - Software speech synthesizer (text-to-speech)
Review Request: espeak - Software speech synthesizer (text-to-speech)
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-10-04 11:14 EDT by Francois Aucamp
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-14 13:06:48 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Francois Aucamp 2006-10-04 11:14:53 EDT
Spec URL: http://abri.homelinux.com/fedora/espeak/espeak.spec
SRPM URL: http://abri.homelinux.com/fedora/espeak/espeak-1.16-1.src.rpm

Description:
eSpeak is a software speech synthesizer for English and other languages.

eSpeak produces good quality English speech. It uses a different synthesis
method from other open source TTS engines, and sounds quite different.
It's perhaps not as natural or "smooth", but some people may find the
articulation clearer and easier to listen to for long periods. eSpeak supports
several languages, however in most cases these are initial drafts and need more
work to improve them.

It can run as a command line program to speak text from a file or from stdin.


This is my second package. I am looking for a sponsor.
Comment 1 Mamoru TASAKA 2006-10-27 22:59:27 EDT
I cannot browse your spec file or download your srpm as
connection gets timeout.
Comment 2 Michel Alexandre Salim 2006-11-01 09:53:28 EST
Ping?

Sounds like an interesting package, but as Mamoru noted, we can't get to it
Comment 3 Francois Aucamp 2006-11-01 10:33:35 EST
Hmm... it seems the person had who graciously provided me with some server space
is having some router problems... :-/ 
I have moved to the spec and SRPM to where I put the flite (#207793) stuff:

Spec URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak.spec
SRPM URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak-1.16-1.src.rpm
Comment 4 Mamoru TASAKA 2006-11-01 12:17:15 EST
Well, just a quick look at this package:

A. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* Filesystem Layout (and others)
  - I think installing header files into /usr/include should be
    avoided, they should be installed under /usr/include/espeak.
  - By the way, why do you install only 'speak_lib.h'? I can find
    other header files.
  - http://espeak.sourceforge.net/index.html
-----------------------------------------------------
The project name speak had already been taken by 
another project on SourceForge (for a Windows TTS front-end) 
so I added a letter 'e' to the front to make eSpeak. For now, 
the program executable remains speak and is referred to 
as such in the documentation.
-----------------------------------------------------
    I think the binary name 'speak' is somewhat troublesome and
    recommend that the name should be 'espeak' (and the related
    documentation should be changed).
  - Check if /usr/share/espeak-data/soundicons/ is required as
    this is a empty directory. 

* BuildRequires:
  - By the way, is 'portaudio' requires for BuildRequires?

* Compiler flags
  - Fedora specific compiler flags are not passed and this leads
    to useless debuginfo rpm.

B. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   = Perhaps nothing, however, I just did a quick look...

C. Other things I have noticed.
* Installation process.
  - Well, I think the installation process of this package is
    somewhat illegible. Especially, the need of explicit description
    of soname in spec file should be avoided as you have to check
    if soname of shared library is changed each time you build this.
    This should be something like:

--------------------------------------------------------
cd src
install -m 0755 libespeak.so.*.*.* %{buildroot}%{_libdir}
ln -sf libespeak.so.*.*.* %{buildroot}%{_libdir}/libespeak.so
/sbin/ldconfig -n %{buildroot}%{_libdir}/
--------------------------------------------------------

* Voice data
  - Well, I cannot verify the license issue of binary voice data
    named *_dict as this is a binary.
    If these data can be reproduced from ascii text files, it should
    do so. By a quick look, 'speak' has a option of '--compile=?'.

    Is it possible to recompile voice data *dict files by this?
    (Note: executing 'speak' binary requires 'portaudio' on BuildRequires.
     Also, by default 'speak --compile=?' needs superuser admin as
     it tries to access /usr/share/espeak so a patch is required to
     make 'speak' binary have the argument of output files)
Comment 5 Michel Alexandre Salim 2006-11-01 12:27:14 EST
Just adding a few things:
- portaudio is a BuildRequire (see Makefile), so that's fine. It's a bit
confusing that there is no portaudio-devel
- Might want to create an 'install' target in the Makefile and do the copy
operations there. This way you can push the changes upstream. Probably put the
RPM_OPT_FLAGS changes in a separate diff since that's Fedora-specific
- Tested on x86_64

Has anyone sponsored you yet? I can't do it myself, but you might want to
mention it on the mailing list
Comment 6 Mamoru TASAKA 2006-11-01 12:32:35 EST
Well, I am a membership of sponsors and I can sponsor Francois if
I can judge it is preferable.
Comment 7 Michael Schwendt 2006-11-01 17:04:54 EST
"portaudio-devel" is a virtual sub-package of "portaudio", so you
can do "BuildRequires: portaudio-devel" just fine and ought to prefer
doing it.
Comment 8 Francois Aucamp 2006-11-02 13:43:57 EST
Thanks for the feedback! 

New build:
Spec URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak.spec
SRPM URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak-1.16-2.src.rpm

Changes:
- Added "install" target to makefile (makefile_install_target.patch)
- Added patch to fix AMD64 sizeof(char *) assumption bug (upstream request ID
1588938)
- Changed "portaudio" BuildRequires to "portaudio-devel"
- Added patch to makefile to allow RPM_OPT_FLAGS
- Added patch to replace all references to "speak" binary with "espeak"
- Moved header files to /usr/include/espeak

A few comments:
Development headers: As mentioned in the ReadMe, the speak_lib.h provides the
entire API to the libespeak shared library, and it references no other
espeak-specific headers, so it is unecessary to include any other .h files. 

Binary voice data: The espeak program itself (formerly "speak" ;-) ) cannot
compile the binary voice data (using the --compile arg) from source without a
binary version of the phoneme tables being present. These phoneme table data
files cannot be compiled from source using espeak/speak; they are compiled with
a seperate program, "espeakedit", which is an interactive, GUI-based editing
tool, also released under the GPL. There is no explicit license file for the
binary voice data/phoneme tables, but since the source from which these are
compiled is under the GPL, I don't think there are any legal problems.

Patches: Depending on the feedback from this package build, I will push the
makefile patches upstream (except for the RPM_OPT_FLAGS patch). 

espeak name: I agree that the "speak" name is troublesome, and have removed it
from this rpm, as per suggestion. However, we must remember that some other
applications may already be using eSpeak via the "speak" executable (especially
since the shared library is a relatively new addition to espeak); this patch may
break compatability with such programs. Some HOWTO's and guides on the Internet
will also be (very slightly) incompatible with this naming scheme. There are
ways around this, naturally, but I'm uncertain whether changing the name in the
Fedora package is the best course of action. Nevertheless, depending on the
feedback here, I will push upstream for the name change... :-)

I've built this package in mock on FC6/i386. rpmlint is silent, except for the
no-documentation stuff for the -devel subpackage.
Comment 9 Mamoru TASAKA 2006-11-03 10:45:20 EST
Well, a lot of improvements!!

The left things are:
1. From http://fedoraproject.org/wiki/Packaging/Guidelines :

* Use rpmlint
  - Now -debuginfo rpm is correctly created, however, this
    package (-debuginfo) bears lots of complaint, e.g.

E: espeak-debuginfo script-without-shebang \
      /usr/src/debug/espeak-1.16-source/src/sintab.h

    This is due to incorrect permission. Please change this
    to 0644.

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   (= Nothing).

3. Other things I have noticed :
*  Well, I recommend changing suffix for different patches.

*  It seems that makefile_rpmoptflags.patch is not necessary,
   which can be replaced with:
   make %{?_smp_mflags} BIN_NAME=espeak CXXFLAGS=${RPM_OPT_FLAGS}

*  The original files .orig files e.g.
   /usr/share/doc/espeak-1.16/html/add_language.html.orig
   are not necessary.

= Well, 
  * the license issue of binary voice data
  * file list of -devel package 
  are okay.

--------------------------------------------------------------
Please fix above. After that, I will accept this package
and sponsor you.
Comment 10 Francois Aucamp 2006-11-04 12:21:14 EST
New build:
Spec URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak.spec
SRPM URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak-1.16-3.src.rpm

Changes:
- Fixed source file permissions for -debuginfo package in %prep
- Added RPM_OPT_FLAGS to "make" command in %build; removed RPM_OPT_FLAGS
makefile patch
- Modified makefile install target patch to include general support for setting
compiler optimization flags via CXXFLAGS
- Removed creation of .orig backup files during patching
- Modified patch files to have different suffixes


This time, rpmlint is truly silent ;-) I'm not too sure what you mean by
"different suffixes" for different patch files; I've resorted to using a suffix
corresponding to the patch ID number in the spec (e.g. somefile.patch0).
Thanks for the feedback!
Comment 11 Mamoru TASAKA 2006-11-04 12:29:43 EST
Well, what I meant was adding some different suffixes each time you apply patches
are preferred and I didn't meant you have to change the name of the patches.

Like:

%patch0 -p1 -b .pthread
%patch1 -p1 -b .install_target
%patch2 -p1 -b .sizeof
........

to make it easy that you can check how the patches were applied afterwards.

(I have not yet rebuilt the srpm you re-uploaded
Comment 12 Mamoru TASAKA 2006-11-05 09:08:09 EST
Well, it seems that I cannot access http://dialogpalette.sourceforge.net/.

Note: I will be busy till this Thursday (in Japan: EST+14h) and I may not
be able to check your srpm by that day.
Comment 13 Francois Aucamp 2006-11-07 11:49:31 EST
Thanks for the suffix clarification. :-)
I've bumped the release, and did a few minor fixes:
New build:
Spec URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak.spec
SRPM URL: http://dialogpalette.sourceforge.net/extras/fedora/espeak-1.16-4.src.rpm

Changes:
- Modified patch steps to create backups with different suffixes
- Renamed patch file extensions to .patch
- Added step in %install to remove patch backup files in documentation
Comment 14 Mamoru TASAKA 2006-11-08 11:55:19 EST
Final check.
Comment 15 Mamoru TASAKA 2006-11-08 12:14:27 EST
Okay.
--------------------------------------------------------------
  This package (espeak) is APPROVED by me.
--------------------------------------------------------------

Please go forward according to 

http://fedoraproject.org/wiki/Extras/Contributors

to import this package to Fedora Extras. I will sponsor
you when you have taken steps partway written in the page above
(then I should receive the mail that you need a sponsor
 around 6h EST).
Comment 16 Francois Aucamp 2006-11-09 12:52:37 EST
Thanks for the review and sponsorship!

I have imported the package and requested branches for FC-5 and FC-6.
Comment 17 Mamoru TASAKA 2006-11-09 13:13:09 EST
Okay, I saw by mail that you successfully imported espeak
to FE-devel CVS branch.

Well, usually SyncNeeded process may take some time (one or two day)
to be completed by the admin of CVS. Before waiting it (perhaps now),
you should be able to send a build queue for FE-devel. Try it accroding
to http://fedoraproject.org/wiki/Extras/Contributors .

When you successfully built espeak on FE-devel, you can close
this package as "CLOSED NEXTRELEASE" (some people suspend to close
the review request till rebuild for all branches are done, however
I think that you can close this when
* rebuild for FE-devel is done
* SyncNeeded is requested for other branches )

------------------------------------------------
NOTE: please don't forget to add espeak to comps/comps-fe?.xml.in
when building espeak is done, see:
http://fedoraproject.org/wiki/Extras/CompsXml

     and... please also check my review for flite (bug 207793).
Comment 18 Mamoru TASAKA 2006-11-13 04:35:32 EST
By the way, have you tried to rebuild this package
on FE buildsys?
Comment 19 Francois Aucamp 2006-11-13 04:45:28 EST
Yes, but the plague-client build request itself is denied with the following error:
"Server returned an error: Insufficient privileges.". 

All of by client-side setup seems to be fine, so following advice from
fedora-maintainers, I have requested assistance from the Fedora Infrastructure
team (created a ticket in their bugtracker), and am now waiting for a response.
Comment 20 Francois Aucamp 2006-11-14 13:06:48 EST
Ok, build for FE-devel succeeded; FC-5 and FC-6 building in progress.
Closing review request. Thanks to all for helping out during the review!

Note You need to log in before you can comment on or make changes to this bug.