Speech dispatcher rate conversion is wildly inaccurate

VERIFIED FIXED in Firefox 48

Status

()

VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

unspecified
mozilla48
Unspecified
Linux
Points:
---

Firefox Tracking Flags

(firefox48 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
With some experimentation, it seems that the rate values given to speechd bring very inaccurate results. From looking at how Chromium converts the rate values, it is apparent that the min/max speeds in speechd are 3x.

Some normalization needs to happen to make this consistent with other platforms browsers.
(Assignee)

Updated

3 years ago
Blocks: 1254234
Could you explain the log10 usage here a bit?
Flags: needinfo?(eitan)
Attachment #8728133 - Flags: review?(bugs) → review+
Comment on attachment 8728133 [details]
MozReview Request: Bug 1254738 - Fix utterance rate conversion for speech dispatcher. r?smaug

https://reviewboard.mozilla.org/r/38809/#review36183

::: dom/media/webspeech/synth/speechd/SpeechDispatcherService.cpp
(Diff revision 1)
> -  // speech-dispatcher expects -100 to 100 with 0 being default.
> -  int rate = 0;
> +  // The rate range in speechd is roughly x3 at both ends (0.334 .. 3).
> +  int rate = std::max<float>(std::min<float>(aRate, 3), 0.334);
>  
> -  if (aRate > 1) {
> -    rate = static_cast<int>((aRate - 1) * 10);
> +  // speech-dispatcher expects -100 to 100 with 0 being default.
> +  spd_set_voice_rate(mSpeechdClient, log10(rate) / log10(3) * 100);
> -  } else if (aRate <= 1) {

So could you at least change the comment about "-100 to 100" a bit explaining what the limits are and that we end up passing from -43 to 43
(Assignee)

Comment 4

3 years ago
Sorry for not being responsive. I took that math from Chromium. That is how they translate rate values. It gives more correct rate scaling, but I think we could do better, specifically at rates that are smaller than 1x.

I made a little tool that can be used to compare speech rates across platforms/voices/browsers. I want to have as much consistency as possible, even though I know getting it fully accurate is impossible.

I'll come up with another translation and flag you for review soon.
Flags: needinfo?(eitan)
(Assignee)

Comment 5

3 years ago
I'm lost with the new mozreview way of doing things.. not sure how to obsolete the patch correctly. I'll upload another one soon.
I'm more than happy to review in bugzilla the old way ;)
But up to you. If you prefer MozReview, use that.
(Assignee)

Updated

3 years ago
Attachment #8728133 - Attachment is obsolete: true
(Assignee)

Comment 8

3 years ago
These adjustments actually make the speech rate better across all voices.
Attachment #8730910 - Flags: review?(bugs)
(Assignee)

Comment 10

3 years ago
^ that is after the applied patch
Comment on attachment 8730910 [details] [diff] [review]
Normalize speech rate for speech dispatcher.

>+  // We provide a rate of 0.1 to 10 with 1 being default.
I don't understand what this means now. Please fix the comment
Attachment #8730910 - Flags: review?(bugs) → review+
(Assignee)

Comment 12

3 years ago
MozReview-Commit-ID: 79OuII34vz7
Attachment #8730910 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Assignee: nobody → eitan

Comment 14

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f7597cb7ff9b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Verified fixed on Firefox 48 Beta 2, buildID: 20160620091522 (with "media.webspeech.synth.enabled" set to true), using this tool: http://eeejay.github.io/webspeechdemos/rates.html.
Status: RESOLVED → VERIFIED
status-firefox48: fixed → verified
You need to log in before you can comment on or make changes to this bug.