Closed Bug 1126282 Opened 5 years ago Closed 3 years ago

Provide a 1.25x faster video speed option in the video controls

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed
relnote-firefox --- 49+

People

(Reporter: rik, Assigned: jaws)

References

Details

Attachments

(1 file, 3 obsolete files)

The current video speed options don't work very well for spoken audio. 1.5x is too fast. I'd like a video option in between. Currently, I end up using the devtools to set this.
YouTube offers 1.25x for their HTML5 player. Would 1.25x work for you? It would be nice to be consistent with them.
Flags: needinfo?(anthony)
1.25x often feels a bit too fast for me (of course that depends on the speaker). 1.15x feels like not altering the audio too much but shorten the viewing time.

I'd settle for 1.25x because it's better than the current menu. But 1.15x feels better to me.
Flags: needinfo?(anthony)
Attached patch 125speed (obsolete) — Splinter Review
Added 1.25x speed option
Attachment #8561692 - Flags: review?(anthony)
Comment on attachment 8561692 [details] [diff] [review]
125speed

I'm not a peer for this part of the code base so I can't review this. Maybe Jared can?
Attachment #8561692 - Flags: review?(anthony) → review?(jaws)
Comment on attachment 8561692 [details] [diff] [review]
125speed

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

::: browser/base/content/browser-context.inc
@@ +129,5 @@
>                      type="radio"
>                      name="playbackrate"
>                      oncommand="gContextMenu.mediaCommand('playbackRate', 1.5);"/>
> +
> +          <menuitem id="context-media-playbackrate-125x"

This menuitem should go before the context-media-playbackrate-150x menuitem.

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +547,4 @@
>  <!ENTITY mediaPlaybackRate050x.accesskey "S">
>  <!ENTITY mediaPlaybackRate100x.label "Normal Speed">
>  <!ENTITY mediaPlaybackRate100x.accesskey "N">
> +<!ENTITY mediaPlaybackRate125x.label "Average Speed (1.25x)">

"Average" is too close in definition to "normal". We'll want something between "normal" and "high". I asked some other Firefox developers and the feedback that I got was that we should use "Fast (1.25×)" here, and then change mediaPlaybackRate150x.label to "Faster (1.5×)".

mediaPlaybackRate125x.accesskey should change to "F"
mediaPlaybackRate150x.accesskey should change to "a"

When you change mediaPlaybackRate150x.label, you'll also need to rename the entity name to mediaPlaybackRate150x2.label and do the same for the associated accesskey (mediaPlaybackRate150x.accesskey -> mediaPlaybackRate150x2.accesskey).

We change the entity names so that localizers will get notified that they need to retranslate these values. We also keep the prefix the same for the label and the accesskey since they are associated and may need to change.

When you change the entity names, you will need to update browser-context.inc to reference the new entity names.
Attachment #8561692 - Flags: review?(jaws) → feedback+
Summary: 1.15x faster video speed → Provide a 1.25x faster video speed option in the video controls
Anam Alvi: Will you be able to iterate on this?
Flags: needinfo?(anam.alvi)
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → jaws
Attachment #8561692 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Flags: needinfo?(anam.alvi)
Attachment #8749654 - Flags: review?(gijskruitbosch+bugs)
Attached patch Patch (qref'd) (obsolete) — Splinter Review
Attachment #8749654 - Attachment is obsolete: true
Attachment #8749654 - Flags: review?(gijskruitbosch+bugs)
Attachment #8749655 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8749655 [details] [diff] [review]
Patch (qref'd)

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

r=me on code. I have grammar quibbles below.

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +569,5 @@
>  <!ENTITY mediaPlaybackRate050x.label "Slow Motion (0.5×)">
>  <!ENTITY mediaPlaybackRate050x.accesskey "S">
>  <!ENTITY mediaPlaybackRate100x.label "Normal Speed">
>  <!ENTITY mediaPlaybackRate100x.accesskey "N">
> +<!ENTITY mediaPlaybackRate125x.label "Fast Speed (1.25×)">

Maybe I'm weird, but "fast speed" sounds odd to me, because both "fast" and "speed" imply velocity. "High" and "Higher" would then make more sense to me.

AIUI these items are all in a submenu labeled "Play speed". Maybe we can just omit "speed" and "motion" from the labels here, and use "Slow", "Normal", "Fast", "Faster" and "Ludicrous" instead ?
Attachment #8749655 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8749655 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/88e5c19cd98e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Release Note Request (optional, but appreciated)
[Why is this notable]: Users have a more desirable playback rate for watching lectures / speeches
[Suggested wording]: Added ability to play audio and video at 1.25× speed through the context menu.
[Links (documentation, blog post, etc)]: n/a
relnote-firefox: --- → ?
See Also: → 1272411
Added to relnotes for 49: "Play audio and video at 1.25× speed through the context menu"
You need to log in before you can comment on or make changes to this bug.