Intent to Implement and Ship: HTMLMediaElement.preservesPitch

[Thomas Guilbert]

Note: both the pitch-shifting and non-pitch-shifting time stretch algorithms are already implemented and in use in Chrome. This will just expose them in a togglable fashion to the web. Safari and Firefox have already shipped this feature.

Contact emails tgui...@chromium.org Explainer No formal explainers, public discussions: https://github.com/whatwg/html/issues/262 https://github.com/whatwg/html/pull/3881 Design docs/spec Specification: https://github.com/whatwg/html/pull/3881 N/A TAG review This is past the design stage, and already implemented by two other browsers, so no TAG review. Summary This flag determines whether or not the pitch of an audio or video element should be preserved when adjusting the playback rate. Motivation This feature is needed for creative purposes (e.g. pitch-shifting "DJ deck" style applications), and is already supported by Safari and Firefox. It can also prevent the introduction of artifacts from pitch-preserving algorithms at playback speeds very close to 1.00. Risks

Interoperability and Compatibility No risk. We are the 3rd browser to implement it. Firefox: Shipped Edge: No public signals Safari: Shipped Web developers: Positive (https://github.com/whatwg/html/issues/262#issuecomment-302705562) Ergonomics N/A Activation No activation risks. Security No security risks.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)? Yes Is this feature fully tested by web-platform-tests? No There are no WPTs for the moment. It seems like WPTs are where the discussion to add this to the HTML standard fell off. A WPT could be written using a WebAudio pitch detector, such as: https://github.com/cwilso/pitchdetect Link to entry on the Chrome Platform Status https://chromestatus.com/feature/5742134990733312

[Yoav Weiss]

On Thu, Jun 18, 2020 at 1:28 AM 'Thomas Guilbert' via blink-dev <blin...@chromium.org> wrote:

Note: both the pitch-shifting and non-pitch-shifting time stretch algorithms are already implemented and in use in Chrome. This will just expose them in a togglable fashion to the web. Safari and Firefox have already shipped this feature.

Contact emails tgui...@chromium.org Explainer No formal explainers, public discussions:

Generally, for small features, even if there's no formal explainer, it's better to write a few lines describing the feature, what it does and how we expect folks to use it.

 

https://github.com/whatwg/html/issues/262 https://github.com/whatwg/html/pull/3881 Design docs/spec Specification: https://github.com/whatwg/html/pull/3881 N/A TAG review This is past the design stage, and already implemented by two other browsers, so no TAG review. Summary This flag determines whether or not the pitch of an audio or video element should be preserved when adjusting the playback rate. Motivation This feature is needed for creative purposes (e.g. pitch-shifting "DJ deck" style applications), and is already supported by Safari and Firefox. It can also prevent the introduction of artifacts from pitch-preserving algorithms at playback speeds very close to 1.00. Risks

Interoperability and Compatibility No risk. We are the 3rd browser to implement it. Firefox: Shipped Edge: No public signals Safari: Shipped Web developers: Positive (https://github.com/whatwg/html/issues/262#issuecomment-302705562) Ergonomics N/A Activation No activation risks. Security No security risks.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)? Yes Is this feature fully tested by web-platform-tests? No There are no WPTs for the moment. It seems like WPTs are where the discussion to add this to the HTML standard fell off. A WPT could be written using a WebAudio pitch detector, such as: https://github.com/cwilso/pitchdetect

Are you planning to add such tests?

 

Link to entry on the Chrome Platform Status https://chromestatus.com/feature/5742134990733312

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABrVPoZHsxgdNOQ8zL3eYtfP6BjQsw_qfNgXZPyRQGQOqkT0dw%40mail.gmail.com.

[Mike West]

I would second Yoav's comments around testing. The fact that two vendors ship this API without substantial WPT coverage means that we likely already have two divergent implementations. It would be excellent to not add a third variant.

Is my intuition incorrect here? Do we have confidence that our implementation matches the shipped implementations in the wild?

-mike

[Thomas Guilbert]

I am planning to add tests right after the flag is implemented (maybe within the same CL). I was optimistically hoping to get the implementation in before the M85 cutoff.

I think that the API is so small that there is little room for divergence. It only dictates the use of a category of time stretch algorithms (pitch preserving or pitch shifting), and nothing else is said about the algorithms themselves. Different browsers already use different algorithm implementations, which create slightly different results, regardless of the use of this flag (e.g., by default, videos sound slightly different at 0.5x or 2.0x on Chrome and Firefox, despite them both using WSOLA). The only meaningful way this could diverge is whether the flag is on by default or not, but it would be very surprising if the flag wasn't on by default.

So, I will add tests to assure a basic form of compliance across browsers, but they will be simple tests, and I can't think of how our implementation could meaningfully differ from the existing implementations in the wild.

I updated the "explainer" portion of ChromeStatus with these few lines. I was limited by character count, so I will expand this and a link to it, if more details should be added:

Pitch preserving algorithms (PP) allow us to change media's playback rate, without affecting its audio frequencies (e.g. watching a video at 2x without "chipmunks" voices).

However pitch shifting (PS) can be desirable for certain applications (e.g DJing/sampling).

This flag allows web developers to toggle between PS and PP playback.

Playback rates close to 1.00 are often used to adjust for audio/visual drift. At those rates, PS can be preferable, with no artifacts, and acceptable pitch shift.

Thanks!

[Mike West]

The discussion at https://github.com/whatwg/html/pull/3881 points to a few aspects of the change that seem quite testable. For instance:

* "Ideally the standard should also point out that changes to the playbackRate and preservesPitch need to be handled continuously and not interrupt playback (e.g. firefox has the correct behavior, safari not...)"

* "we will have to decide whether the pitch adjustment should affect https://webaudio.github.io/web-audio-api/#mediaelementaudiosourcenode or not, and that can be tested. Checking how that behaves in Firefox and Safari would be a good start."

* "I discovered that changing the .playbackRate with or without .moz/webkitPreservesPitch on MediaElementSourceNode’s source doesn’t seem have any effect in Firefox - but it does in Chrome. It should though, as otherwise we don't have a good mechanism to change the playbackRate and the whole thing kind of becomes pointless."

* "That should mean that we can add tests that verify that the pitch changes depending on the use of preservesPitch"

Likewise, in https://github.com/whatwg/html/issues/262:

* "An open question is when (if) to reset the state. It doesn't look like WebKit or Gecko ever do reset it."

These comments are a few years old, but I assume they're still relevant? +Philip Jägenstedt who weighed in on that early discussion.

-mike

[Thomas Guilbert]

Thanks for bringing these snippets up! I will work on those tests next week.

[Thomas Guilbert]

The implementation has landed behind a flag in M85 (here).

I've written tests that verify that Chrome's implementation works as expected (here). Those tests are based on WebAudio, and won't work with Firefox's current implementation of preservesPitch (I've opened this bug and will see what they say). I wasn't able to verify on Safari, since I don't have a Mac and createMediaElementSource is currently broken (but this bug says it is fixed in beta).

The tests check the default state of the flag, the interaction with MediaElementAudioSourceNode, and that the pitch does/doesn't change as expected when the flag is on/off.

I also manually confirmed that Chrome's implementation plays continuously, and tried writing a discontinuity verification test still using WebAudio. It successfully detects interruptions/silences, but it cannot detect jumps in the playback's waveform. To do so would require some more advanced signal processing, which I don't know much about and I'm afraid would be flaky. I'm not even sure that the silence detector test is worth keeping.

Of the points brought up above, this only leaves the question of resetting state. I don't think there is anything to test here.

There is also technically no test that verifies the pitch behavior of a media element without MediaElementAudioSourceNode, but I don't know how one could programmatically do that.

Is there anything else that needs to be considered? Is the silence detector test worth keeping?

Thanks,
Thomas

[Mike West]

Thanks, Thomas! I'm happy to see you working through improving the test coverage, and this takes care of my concerns with the intent. LGTM1.

For the detail questions about what kinds of tests make sense, I'd defer to you, and suggest that you reach out to folks working on other engines to probe for potential interop issues.

-mike

[Daniel Bratell]

LGTM2

/Daniel

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKXHy%3DfJBZiGNnxOjerf379s4k-SBq8mpMBk%2BK70hSLb%2B3OoBA%40mail.gmail.com.

[Manuel Rego Casasnovas]

Are we doing the tests in WPT or only as Chromium internal tests?

If it's not possible to do them in WPT we should at least file an issue
there (https://github.com/web-platform-tests/wpt/issues), so this can be
done in the future.
Otherwise internal tests are not going to be helpful regarding
interoperability between implementations.

Bye,
Rego

On 30/06/2020 12:33, Daniel Bratell wrote:
> LGTM2
>
> /Daniel
>
> On 2020-06-30 09:02, Mike West wrote:
>> Thanks, Thomas! I'm happy to see you working through improving the
>> test coverage, and this takes care of my concerns with the intent. LGTM1.
>>
>> For the detail questions about what kinds of tests make sense, I'd
>> defer to you, and suggest that you reach out to folks working on other
>> engines to probe for potential interop issues.
>>
>> -mike
>>
>>
>> On Sat, Jun 27, 2020 at 1:54 AM Thomas Guilbert <tgui...@google.com
>> <mailto:tgui...@google.com>> wrote:
>>
>> The implementation has landed behind a flag in M85 (here

>> <https://chromium.googlesource.com/chromium/src/+/dd0e3e67a3dc240efd2eab19d6496de0319b6118>).

>>
>> I've written tests that verify that Chrome's implementation works
>> as expected (here

>> <https://chromium-review.googlesource.com/c/chromium/src/+/2259375>).

>> Those tests are based on WebAudio, and won't work with
>> Firefox's current implementation of preservesPitch (I've opened
>> this bug

>> <https://bugzilla.mozilla.org/show_bug.cgi?id=1648277> and will

>> see what they say). I wasn't able to verify on Safari, since I
>> don't have a Mac and createMediaElementSource is currently broken
>> (but this bug

>> <https://bugs.webkit.org/show_bug.cgi?id=211394> says it is fixed

>> <mailto:foo...@chromium.org> who weighed in on that early

>> yo...@yoav.ws <mailto:yo...@yoav.ws> wrote:
>>
>> On Thu, Jun 18, 2020 at 1:28 AM 'Thomas
>> Guilbert' via blink-dev <blin...@chromium.org>
>> wrote:
>>
>> Note: both the pitch-shifting and
>> non-pitch-shifting time stretch algorithms
>> are already implemented and in use in
>> Chrome. This will just expose them in a
>> togglable fashion to the web. Safari and
>> Firefox have already shipped this feature.

>> *Contact
>> emails*tgui...@chromium.orgExplainerNo
>> formal explainers, public discussions:
>> <https://chromestatus.com/admin/features/launch/No%20formal%20explainers,%20public%20discussions:>

>>
>>
>> Generally, for small features, even if there's
>> no formal explainer, it's better to write a
>> few lines describing the feature, what it does
>> and how we expect folks to use it.
>>  
>>
>> https://github.com/whatwg/html/issues/262https://github.com/whatwg/html/pull/3881Design
>> docs/specSpecification:

>> https://github.com/whatwg/html/pull/3881N/A <https://chromestatus.com/admin/features/launch/N/A>TAG
>> reviewThis is past the design stage, and

>> already implemented by two other browsers,

>> so no TAG review. SummaryThis flag

>> determines whether or not the pitch of an
>> audio or video element should be preserved
>> when adjusting the playback rate.

>> MotivationThis feature is needed for

>> creative purposes (e.g. pitch-shifting "DJ
>> deck" style applications), and is already
>> supported by Safari and Firefox. It can
>> also prevent the introduction of artifacts
>> from pitch-preserving algorithms at
>> playback speeds very close to 1.00. Risks
>> Interoperability and Compatibility No
>> risk. We are the 3rd browser to implement

>> it. /Firefox/: Shipped /Edge/: No public
>> signals /Safari/: Shipped /Web
>> developers/: Positive

>> (https://github.com/whatwg/html/issues/262#issuecomment-302705562)
>> Ergonomics N/A Activation No activation
>> risks. Security No security risks.
>> Will this feature be supported on all six
>> Blink platforms (Windows, Mac, Linux,
>> Chrome OS, Android, and Android

>> WebView)?Yes Is this feature fully tested
>> by web-platform-tests
>> <https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md>?No

>> There are no WPTs for the moment. It seems
>> like WPTs are where the discussion to add
>> this to the HTML standard fell off. A WPT
>> could be written using a WebAudio pitch
>> detector, such as:
>> https://github.com/cwilso/pitchdetect
>>
>>
>> Are you planning to add such tests?
>>  
>>
>> Link to entry on the Chrome Platform

>> Statushttps://chromestatus.com/feature/5742134990733312

>>
>> --
>> You received this message because you are
>> subscribed to the Google Groups
>> "blink-dev" group.
>> To unsubscribe from this group and stop
>> receiving emails from it, send an email to
>> blink-dev+...@chromium.org.
>> To view this discussion on the web visit
>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABrVPoZHsxgdNOQ8zL3eYtfP6BjQsw_qfNgXZPyRQGQOqkT0dw%40mail.gmail.com

>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABrVPoZHsxgdNOQ8zL3eYtfP6BjQsw_qfNgXZPyRQGQOqkT0dw%40mail.gmail.com?utm_medium=email&utm_source=footer>.

>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups "blink-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to blink-dev+...@chromium.org

>> <mailto:blink-dev+...@chromium.org>.

>> To view this discussion on the web visit

>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKXHy%3DfJBZiGNnxOjerf379s4k-SBq8mpMBk%2BK70hSLb%2B3OoBA%40mail.gmail.com
>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKXHy%3DfJBZiGNnxOjerf379s4k-SBq8mpMBk%2BK70hSLb%2B3OoBA%40mail.gmail.com?utm_medium=email&utm_source=footer>.

>
> --
> You received this message because you are subscribed to the Google
> Groups "blink-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to blink-dev+...@chromium.org

> <mailto:blink-dev+...@chromium.org>.

> To view this discussion on the web visit

> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/1205840e-82f4-df18-a608-717dada1626f%40gmail.com
> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/1205840e-82f4-df18-a608-717dada1626f%40gmail.com?utm_medium=email&utm_source=footer>.

[Thomas Guilbert]

I had the intention of submitting the tests as WPTs, but they definitely will fail on Firefox until the "MediaElementAudioSourceNode should take into account preserves pitch" bug is fixed.

[Yoav Weiss]

Failing tests are a good reminder for people to implement or align...

LGTM3

[Thomas Guilbert]

Thank you very much everyone!

[Thomas Guilbert]

Quick update -- the feature is now on by default:

https://chromium.googlesource.com/chromium/src/+/f3ec10b358a091faeaa2c6c9586fe9ae2bb769a4

Thanks!