Closed Bug 1225347 Opened 5 years ago Closed 5 years ago

Sepak() method should apply current window's volume setting

Categories

(Core :: Web Speech, defect)

Unspecified
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

References

Details

Attachments

(1 file)

Android Speech API (bug 1184142) can only set volume when calling speak API.  So after utterance, it cannot change it.  So ,we should set the volume when calling API.

Current implementation is
1. Call Speak()
2. Call OnVolumeChanged() to apply the volume of current window

We should change like the following.
1. Call Speak() with applying the volume of current window
Blocks: 1184142
Attachment #8692328 - Attachment description: speech-audio-improvement → Apply audio setting to volume parameter of Speak()
Comment on attachment 8692328 [details] [diff] [review]
Apply audio setting to volume parameter of Speak()

Now, volume setting on tab is applied on start event.  But it may be too late to set volume.

- Android TTS API cannot change volume after calling speaking().
- Windows SAPI backend can change volume per boundary.  So even if muted, synthesis may output voice until 1st boundary.

So volume should apply when calling speak().
Attachment #8692328 - Flags: review?(eitan)
I'm worried that this doesn't work in e10s correctly. It will always get the volume of the top-level root window. Don't you want the volume of the content window? This may even need to happen in SpeechSynthesis.cpp.
Flags: needinfo?(m_kato)
(In reply to Eitan Isaacson [:eeejay] from comment #3)
> I'm worried that this doesn't work in e10s correctly. It will always get the
> volume of the top-level root window. Don't you want the volume of the
> content window? This may even need to happen in SpeechSynthesis.cpp.

Actually, volume control by tab doesn't work for speech synthesis with e10s (I will work it by bug 1222697).  To get volume status on tab that runs on content process, we should get it on content process.  Of course, this fix code works even if e10s.

And, when creating AudioChannelAgent on nsSpeechTask, it uses mUtterance.GetOwner(). so it uses top-level window too.  Incorrect?
Flags: needinfo?(m_kato)
Comment on attachment 8692328 [details] [diff] [review]
Apply audio setting to volume parameter of Speak()

Review of attachment 8692328 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay. It took me a while to understand how this works in e10s at all.
Attachment #8692328 - Flags: review?(eitan) → review+
https://hg.mozilla.org/mozilla-central/rev/9e904a370af3
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.