bpierre / gtranslate

Translate the selected text using Google Translate.
https://addons.mozilla.org/firefox/addon/gtranslate/
Mozilla Public License 2.0
95 stars 27 forks source link

Add text-to-speech feature #63

Open fluks opened 8 years ago

fluks commented 8 years ago

Hi, I added an option to listen to Google Translate's TTS voice. Works on the Firefox developer edition(46.0a2), I can't test it on the main release because of the extension signature enforcement, even I have 'xpinstall.signatures.required' in about:config, still won't install if I toggle it to false.

To listen to the voice you need to press the Listen label.

PerfectSlayer commented 8 years ago

Thanks for your contribution! I'll try to test it this weekend if no one validate it before ;)

fluks commented 8 years ago

I was able to install it to the normal version of Firefox now.

The first time you listen the speech it works, but after that it doesn't. Strange that in the developer version I didn't notice this behavior. It looks like a new AudioContext must be created every time playing the speech. I wonder if the audio thing might leak resources if you don't clean up after it every time. Is there any good way to test for memory leaks in FF?

fluks commented 8 years ago

Now, a new AudioContext is created every time when you want to listen to the speech. I'll try to research about freeing up resources afterwards, is it needed at all. And about the Web Audio API in general.

I have been running this code all day on my browser and it seems to work fine, but I'd like to know for sure it's correct.

PerfectSlayer commented 8 years ago

Sad to see AudioContext behavior is not the same accros the Firefox versions.. Feel free to share a link to your signed dev version to allow us to test on current the Firefox :wink:

bpierre commented 8 years ago

Thanks @fluks, looks great!

The AudioContext should be freed automatically by the garbage collector if there is no references to it? You can use this tool to debug memory in Firefox: https://developer.mozilla.org/en-US/docs/Tools/Memory

fluks commented 8 years ago

PerfectSlayer: Sad to see AudioContext behavior is not the same accros the Firefox versions.. Feel free to share a link to your signed dev version to allow us to test on current the Firefox :wink:

It should work now on the current version of Firefox and on the developer version also. You can test the feature by just pulling my branch? I haven't signed anything yet, do I really need to?

I was able to install an unsigned extension by:

Installing an unsigned extension via url bar by pointing it to the location of the extension on the file system failed.

bpierre: The AudioContext should be freed automatically by the garbage collector if there is no references to it?

I'll remove the AudioBufferSourceNode.onended callback altogether and see what happens

fluks commented 8 years ago

Here's a signed xpi for you to test, @PerfectSlayer, if you still need it.

https://github.com/fluks/gtranslate/raw/xpi/gtranslate-0.13.0-fx%2Ban.xpi

PerfectSlayer commented 8 years ago

fluks: It should work now on the current version of Firefox and on the developer version also. You can test the feature by just pulling my branch? I haven't signed anything yet, do I really need to?

Thanks. I thought they already remove the xpinstall.signatures.required setting (but no !).

By the way, it seems to work fine! :clap: If I could suggest an improvement, it's to stop the current AudioContext if a new one is started. Otherwise, you will have the voices that overlap.

PerfectSlayer commented 8 years ago

So, is-it stable enough? Does anyone test-it? @bpierre Do you consider approving it?

bpierre commented 8 years ago

I haven’t tested it yet, but I definitely consider to approve it.

Some things I see:

fluks commented 8 years ago

So, is-it stable enough? Does anyone test-it?

It has been in my tests. I have been using a version with a feature you suggested, such that the voices won't overlap. I haven't committed this version.

fluks commented 8 years ago

It should be made optional for the provider to support this feature. I plan to add Bing at some point, and I am not sure they will have this feature.

The translator page can speak and the official API also has this feature. https://msdn.microsoft.com/en-us/library/ff512405.aspx

For the same portability reasons, what would you think of moving the XHR part, and maybe the playing part too, inside of the provider (providers/google-translate.js? That way, index.js would just ask the provider to “play the translation” if the feature is supported, all the details belong to the provider itself.

Makes sense, I'll do that.

I don't know is this feature wanted by many users, but the change in the ui is quite minor currently. Here's a pic.

fluks commented 8 years ago

This code doesn't address the overlapping problem. To stop the audio being played, you need to call AudioBufferSourceNode.stop(). So, that state should be saved somewhere (source variable). Maybe the providers should be classes?

I also removed some unnecessary code.