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.

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

Allow to link against upstream espeak-ng #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leso-kn
Copy link
Contributor

@leso-kn leso-kn commented Jul 11, 2023

This PR allows to link piper-phonemize with the unmodified upstream version of espeak-ng.

Tested and working on glibc and musl.

@leso-kn leso-kn changed the title Allow to link with upstream espeak-ng Allow to link against upstream espeak-ng Jul 11, 2023
@leso-kn
Copy link
Contributor Author

leso-kn commented Jul 11, 2023

Following up on a related discussion over here, this PR was changed to a more robust flow, the original implementation can be reviewed on this branch.

@synesthesiam
Copy link
Contributor

Unless I'm missing something, this seems to not preserve punctuation (which is the reason for the espeak-ng fork). The problem is that espeak_TextToPhonemes does not return punctuation with phonemes. This is important for Piper, because voices use punctuation to correctly pause and for correct intonation.

The previous solution looked like it would do the right thing, despite being less robust.

@leso-kn
Copy link
Contributor Author

leso-kn commented Jul 12, 2023

Almost :) Both solutions do preserve punctionation, the difference is that the new solution does not outsource punctuation from espeak-ng, thus providing a hack-free solution at the cost of being marginally less flexible.

However, I did check the espeak-ng punctuation code and for the cases handled by piper-phonemize the chosen manual implementation should not be signifficantly less complete.

@synesthesiam
Copy link
Contributor

My tests fail though when using this PR and espeak-ng 1.51. Have they changed things in a later release?

@leso-kn
Copy link
Contributor Author

leso-kn commented Jul 26, 2023

Sorry for the late reply, work came along!

Indeed, the new implementation was not handling the case where inputTextPointer was already NULL – that didn't happen during my tests because I tested through stdin, thus always had a \n behind my test phrases.

Should be fixed with c46ba6e, test.cpp is now passing on my side.

@synesthesiam
Copy link
Contributor

Great, I'll check again. Thanks!

@leso-kn leso-kn force-pushed the espeak-compat branch 2 times, most recently from 9c6c016 to 80ad15e Compare August 8, 2023 11:41
@leso-kn
Copy link
Contributor Author

leso-kn commented Aug 11, 2023

@synesthesiam FYI, PR was rebased on the latest master branch :)

@bzp83
Copy link

bzp83 commented Apr 18, 2024

is this going to be merged?

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.

None yet

3 participants