crow-translate / QOnlineTranslator

A library for Qt5 that provides free usage of Google, Yandex and Bing translate API.
GNU General Public License v3.0
79 stars 12 forks source link

Added regional voice support for Google #43

Closed Crissium closed 1 year ago

Crissium commented 1 year ago

close #40

Example:

QMediaPlayer player;
QMediaPlaylist list;
QOnlineTts tts;
tts.setRegionPreferences({{QOnlineTranslator::English, QOnlineTts::EnglishUk}});
tts.generateUrls("squirrel", QOnlineTranslator::Google, QOnlineTranslator::English);
list.addMedia(tts.media());
player.setPlaylist(&list);
player.play();

I get the list of supported regional voices from the official Google Translate Android app. Turns out that it is not very impressive.

If Google encounters an unsupported region code, it won't return error; instead, it defaults to the region-neutral language code.

PS: It is my first major contribution to others' repo. Please have the goodness to tell me if I have botched it and if there's anything improperly done (for example, breach of coding style)!

Crissium commented 1 year ago

Shall I close the original PR?

Shatur commented 1 year ago

Shall I close the original PR?

Yes, please :)

Crissium commented 1 year ago

Yes, I'll go along with that idea. I wrote this enum because I didn't know QLocale class before (haven't done any i18n). QLocale::Country is indeed very suitable here. So a big rewrite is in order now. But I've got plenty of spare time in hand.

And one more thing:

QString QOnlineTts::languageApiCode(QOnlineTranslator::Engine engine, QOnlineTranslator::Language lang)
{
    switch (engine) {
    case QOnlineTranslator::Google:
    case QOnlineTranslator::Lingva: // Lingva is a frontend to Google Translate
        if (lang != QOnlineTranslator::Auto) {
            if (m_regionPreferences.contains(lang))
                return regionCode(m_regionPreferences.value(lang));
            else
                return QOnlineTranslator::languageApiCode(engine, lang); // Google use the same codes for tts (except 'auto')
        }
        break;

Should we do a test here checking for DefaultRegion like this:

if (m_regionPreferences.contains(lang) && m_regionPreferences.value(lang) != DefaultRegion)
// ...

This way DefaultRegion will really default to the corresponding language code without the region specifier.

Shatur commented 1 year ago

So a big rewrite is in order now. But I've got plenty of spare time in hand.

Nice to hear!

Should we do a test here checking for DefaultRegion like this:

What if we use QMap::value and pass QOnlineTranslator::languageApiCode call as a default value (second argument)?

Crissium commented 1 year ago

Oh, yes. Silly me. Just forgot QMap has such a nice feature.

Crissium commented 1 year ago

Sorry, just removed a review request. I have no idea how it got removed when I only re-requested a review.

Shatur commented 1 year ago

@Crissium if you are interesting to see this feature in Crow soon, I would appreciate a PR :smile: Let me know if you are interested. Feel free to ask me any questions about it, I will walk you through the code base (it's small).

Crissium commented 1 year ago

Yes, I am perfectly willing to contribute to Crow! I've got 10 days with few things to do.

Crissium commented 1 year ago

Sorry, when I'm adding the region option to Crow, it turns out that the regionName method duplicates the language name ("Bangla: Bangla (Bangladesh)" sounds stupid), which it shouldn't. I shall open a new PR and submit the fix. Sorry for the trouble :(