Commit 7b6c4007 authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Commit Bot

Deflake preservesPitch tests

This CL changes preservesPitch tests to wait until the audioElement's
currentTime is at least 0.5 seconds before trying to detect the pitch.
Without this check, the test can try to detect the pitch before the
audio has played enough, which can return a dominant frequency of 0Hz
and fail the test.

The CL also makes a few stylistic changes, and fixes an off-by-one error
in the number of increments used to calculate frequencies.

Bug: 1096238
Change-Id: I6e98e172862a47bea1c4026737138293914f7906
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2298281
Auto-Submit: Thomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: default avatarPhilip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788535}
parent 0010a8da
...@@ -16,14 +16,15 @@ function getPitchDetector(media, t) { ...@@ -16,14 +16,15 @@ function getPitchDetector(media, t) {
sourceNode.connect(analyser); sourceNode.connect(analyser);
analyser.connect(audioContext.destination); analyser.connect(audioContext.destination);
// Returns the frequency value for the nth FFT bin. return () => getPitch(analyser);
var binConverter = (bin) => audioContext.sampleRate*(bin/FFT_SIZE);
return () => getPitch(analyser, binConverter);
} }
function getPitch(analyser, binConverter) { function getPitch(analyser) {
var buf = new Uint8Array(FFT_SIZE/2); // Returns the frequency value for the nth FFT bin.
var binConverter = (bin) =>
(analyser.context.sampleRate/2)*((bin)/(analyser.frequencyBinCount-1));
var buf = new Uint8Array(analyser.frequencyBinCount);
analyser.getByteFrequencyData(buf); analyser.getByteFrequencyData(buf);
return findDominantFrequency(buf, binConverter); return findDominantFrequency(buf, binConverter);
} }
...@@ -40,9 +41,8 @@ function findDominantFrequency(buf, binConverter) { ...@@ -40,9 +41,8 @@ function findDominantFrequency(buf, binConverter) {
} }
} }
// The distance between bins is always constant and corresponds to // The spread of frequencies within bins is constant and corresponds to
// (1/FFT_SIZE)th of the sample rate. Use the frequency value of the 1st bin // (1/(FFT_SIZE-1))th of the sample rate. Use the value of bin #1 as a
// as the margin directly, instead of calculating an average from the values // shorthand for that value.
// of the neighboring bins.
return { value:binConverter(bin), margin:binConverter(1) }; return { value:binConverter(bin), margin:binConverter(1) };
} }
\ No newline at end of file
...@@ -56,27 +56,34 @@ function testPreservesPitch(preservesPitch, playbackRate, expectedPitch, descrip ...@@ -56,27 +56,34 @@ function testPreservesPitch(preservesPitch, playbackRate, expectedPitch, descrip
audio.playbackRate = playbackRate; audio.playbackRate = playbackRate;
setPreservesPitch(audio, preservesPitch); setPreservesPitch(audio, preservesPitch);
function promiseTimeUpdate() { function waitUntil(time) {
return new Promise((resolve) => audio.ontimeupdate = resolve); return new Promise((resolve) => {
audio.ontimeupdate = () => {
if (audio.currentTime >= time) {
resolve();
}
};
});
} }
function verifyPitch() { await test_driver.bless("Play audio element", () => audio.play() )
var pitch = detector();
// 25Hz is larger than the margin we get from 48kHz and 44.1kHz // Wait until we have at least played some audio. Otherwise, the
// audio being analyzed by a FFT of size 2048. If we get something // detector might return a pitch of 0Hz.
// different, there is an error within the test's calculations (or await waitUntil(0.5);
// we might be dealing a larger sample rate).
assert_less_than(pitch.margin, 25,
"Test error: the margin should be reasonably small.")
assert_approx_equals(pitch.value, expectedPitch, pitch.margin, var pitch = detector();
"The actual pitch should be close to the expected pitch.");
} // 25Hz is larger than the margin we get from 48kHz and 44.1kHz
// audio being analyzed by a FFT of size 2048. If we get something
// different, there is an error within the test's calculations (or
// we might be dealing a larger sample rate).
assert_less_than(pitch.margin, 25,
"Test error: the margin should be reasonably small.")
assert_approx_equals(pitch.value, expectedPitch, pitch.margin,
"The actual pitch should be close to the expected pitch.");
await test_driver.bless("Play audio element", () => audio.play() )
.then(promiseTimeUpdate)
.then(verifyPitch);
}, description); }, description);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment