Closed Bug 1446346 Opened 2 years ago Closed 2 years ago

Intermittent dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0.8999999985098839, first bad index: 0 with test-data offset 0 and expected-data offset 0; corresponding values 1 and 0.10000000149011612

Categories

(Core :: Web Audio, defect, P5)

60 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- fixed
firefox59 --- unaffected
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: tjr)

References

Details

(Keywords: intermittent-failure, regression, Whiteboard: [stockwell disabled][Pernosco])

Attachments

(1 file, 5 obsolete files)

Filed by: ncsoregi [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=168449240&repo=mozilla-central

https://queue.taskcluster.net/v1/task/BYbI5lN3QXG1jwADghD7nw/runs/0/artifacts/public/logs/live_backing.log

04:22:27     INFO - TEST-START | dom/media/webaudio/test/test_audioParamLinearRamp.html
04:22:27     INFO - TEST-INFO | started process screencapture
04:22:27     INFO - TEST-INFO | screencapture: exit 0
04:22:27     INFO - Buffered messages logged at 04:22:27
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of channels for expected buffer 0 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of expected frames 
04:22:27     INFO - Buffered messages finished
04:22:27     INFO - TEST-UNEXPECTED-FAIL | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0.8999999985098839, first bad index: 0 with test-data offset 0 and expected-data offset 0; corresponding values 1 and 0.10000000149011612 --- differences - got 1535, expected +0
04:22:27     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:312:5
04:22:27     INFO -     compareChannels@dom/media/webaudio/test/webaudio.js:78:3
04:22:27     INFO -     compareBuffers@dom/media/webaudio/test/webaudio.js:103:5
04:22:27     INFO -     testOutput/sp.onaudioprocess@dom/media/webaudio/test/webaudio.js:214:11
04:22:27     INFO -     EventHandlerNonNull*testOutput@dom/media/webaudio/test/webaudio.js:211:9
04:22:27     INFO -     runTestOnContext@dom/media/webaudio/test/webaudio.js:202:9
04:22:27     INFO -     testOnNormalContext@dom/media/webaudio/test/webaudio.js:222:7
04:22:27     INFO -     runTestFunction@dom/media/webaudio/test/webaudio.js:253:5
04:22:27     INFO -     rval@SimpleTest/SimpleTest.js:146:17
04:22:27     INFO -     EventHandlerNonNull*this.addLoadEvent@SimpleTest/SimpleTest.js:171:13
04:22:27     INFO -     @SimpleTest/SimpleTest.js:1434:5
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of channels for expected buffer 0 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of expected frames 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of input buffer channels 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0, first bad index: -1 with test-data offset 0 and expected-data offset 0; corresponding values undefined and undefined --- differences 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of channels for expected buffer 0 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of expected frames 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of input buffer channels 
04:22:27     INFO - TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0, first bad index: -1 with test-data offset 0 and expected-data offset 0; corresponding values undefined and undefined --- differences 
04:22:27     INFO - GECKO(870) | MEMORY STAT | vsize 4246MB | residentFast 236MB | heapAllocated 22MB
04:22:27     INFO - TEST-OK | dom/media/webaudio/test/test_audioParamLinearRamp.html | took 104ms
There have been 42 failures in the last week.

Summary: Intermittent dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0.8999999985098839, first bad index: 0 with test-data offset 0 and expected-data offset 0; corresponding values 1 and 0.10000000149011612
 
Affected platforms and builds: 
-  windows10-64-qr / debug & opt : 13
-  Linux x64 / debug & opt & pgo & asan: 8
-  windows10-64 / pgo & opt: 4
-  Windows 7 / debug & pgo: 4
-  Linux / opt & debug: 4
-  android-4-3-armv7-api16 / debug & opt: 4
-  OS X 10.10 /debug & opt: 3
-  linux64-qr / opt: 2

Recent log file and snippet with the failure:
https://treeherder.mozilla.org/logviewer.html#?repo=autoland&job_id=170940706&lineNumber=61277

01:08:25     INFO -  TEST-INFO | started process screenshot
01:08:25     INFO -  TEST-INFO | screenshot: exit 0
01:08:25     INFO -  Buffered messages logged at 01:08:25
01:08:25     INFO -  1949 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of channels for expected buffer 0
01:08:25     INFO -  1950 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of expected frames
01:08:25     INFO -  Buffered messages finished
01:08:25    ERROR -  1951 INFO TEST-UNEXPECTED-FAIL | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0.8999999985098839, first bad index: 0 with test-data offset 0 and expected-data offset 0; corresponding values 1 and 0.10000000149011612 --- differences - got 5, expected +0
01:08:25     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:312:5
01:08:25     INFO -      compareChannels@dom/media/webaudio/test/webaudio.js:78:3
01:08:25     INFO -      compareBuffers@dom/media/webaudio/test/webaudio.js:103:5
01:08:25     INFO -      testOutput/sp.onaudioprocess@dom/media/webaudio/test/webaudio.js:214:11
01:08:25     INFO -      EventHandlerNonNull*testOutput@dom/media/webaudio/test/webaudio.js:211:9
01:08:25     INFO -      runTestOnContext@dom/media/webaudio/test/webaudio.js:202:9
Flags: needinfo?(drno)
Whiteboard: [stockwell needswork]
when this switches to disable-recommended, we should disable it- this might be related to bug 1446120.
Attached patch Skipped on linux and windows (obsolete) — Splinter Review
Attachment #8964897 - Flags: review?(jmaher)
Comment on attachment 8964897 [details] [diff] [review]
Skipped on linux and windows

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

we might have to end up skipping osx- lets see if this reduces the failures enough.
Attachment #8964897 - Flags: review?(jmaher) → review+
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc34ae9a1867
Disable test_audioParamLinearRamp.html on linux and windows for frequent failures. r=jmaher
Keywords: leave-open
Whiteboard: [stockwell disable-recommended] → [stockwell disabled]
See Also: → 1449421
See Also: → 1449809
See Also: → 1446604
See Also: → 1443991
See Also: → 1449912
See Also: → 1449869
Alex, it looks like in the middle of March a whole set of webaudio tests started failing. I'm hoping that they share a common root cause.

Can I ask you to have an initial look of what could potentially have caused this?
Flags: needinfo?(drno) → needinfo?(achronop)
Assignee: nobody → achronop
Flags: needinfo?(achronop)
This is caused by the fuzzing on timer introduced recently because of Spectre [1].

Tom, this is making the Web Audio API rather hard to test and causes a very large number of intermittents, do you think it's feasible to flip the pref for testing, or would you have any concerns?

[1]: https://searchfox.org/mozilla-central/source/dom/media/webaudio/AudioContext.cpp#673
Flags: needinfo?(tom)
Webaudio logs of this error:
https://taskcluster-artifacts.net/Ef2STjUUREGuGlAgUi7EeA/0/public/logs/live_backing.log
[task 2018-04-10T15:17:33.035Z] 15:17:33     INFO - TEST-START | dom/media/webaudio/test/test_audioParamLinearRamp.html
...
[task 2018-04-10T15:17:33.343Z] 15:17:33     INFO - GECKO(2130) | [2181:Main Thread]: D/WebAudioAPI 0.000100: gain for 168 SetValueAtTime value=0.1 time=0.000100 constant=0
...
[task 2018-04-10T15:17:33.944Z] 15:17:33     INFO - TEST-UNEXPECTED-FAIL | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0.8999999985098839, first bad index: 0 with test-data offset 0 and expected-data offset 0; corresponding values 1 and 0.10000000149011612 --- differences - got 5, expected +0
Whiteboard: [stockwell disabled] → [stockwell disabled][Pernosco]
(In reply to Tom Ritter [:tjr] from comment #22)
> Created attachment 8966627 [details]
> Bug 1446346 Do not jitter time on several webaudio tests
> 
> Review commit: https://reviewboard.mozilla.org/r/235346/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/235346/

Thanks. This should help, but I'm afraid we're playing whack-a-mole here.
Flags: needinfo?(tom)
Comment on attachment 8966627 [details]
Bug 1446346 If an AudioContext hasn't been started, do not bother clamping/jittering the CurrentTime

https://reviewboard.mozilla.org/r/235346/#review241342
Attachment #8966627 - Flags: review?(padenot) → review+
should we reenable the tests on linux/windows as well:
https://hg.mozilla.org/mozilla-central/rev/fc34ae9a1867

Also as a note I see :roc added the [Pernosco] tag to this bug, there is an opportunity to debug this in the Pernosco toolchain as it has been reproduced in there.  That should help reduce the whack-a-mole attempts here.
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #25)
> Also as a note I see :roc added the [Pernosco] tag to this bug, there is an
> opportunity to debug this in the Pernosco toolchain as it has been
> reproduced in there.  That should help reduce the whack-a-mole attempts here.

roc contacted me by email with a Pernosco link for this one, which allowed me to quickly find the cause for this issue, but it was unclear to me how public Persnoco is at the minute so I didn't mention anything.

The style of testing we've adopted to test the Web Audio API in Firefox very often requires that an `OfflineAudioContext` that has not been started via `startRendering` has a `currentTime` of zero. This seems like an invariant we can conserve even with the timer fuzzing.

Now, it's unclear to me at the minute whether this also affects normal `AudioContext`s, maybe they have the same issue. In any case, we can certainly change our code so that the aforementioned invariant still holds.

Tom, thoughts?
Flags: needinfo?(tom)
The WebAudio API defines that currentTime returns a value that is a multiple of 128/sampleRate.
Even for sampleRate of 96k/s, that is 0.0013s.

Is there a need to reduce resolution more than that (outside of the cases where prefs are changed because users do not want to follow the standards)?

RFPOnly is not documented, but looks like is may be what is wanted here?

Would it be sufficient to use that, if we ensure that sampleRate cannot be higher than 96k/s?
Flags: needinfo?(tom)
I wonder why testing/web-platform/tests/webaudio/the-audio-api/the-offlineaudiocontext-interface/current-time-block-size.html is not detecting this flaw.
(In reply to Karl Tomlinson (:karlt) from comment #30)
> The WebAudio API defines that currentTime returns a value that is a multiple
> of 128/sampleRate.
> Even for sampleRate of 96k/s, that is 0.0013s.
> 
> Is there a need to reduce resolution more than that (outside of the cases
> where prefs are changed because users do not want to follow the standards)?
> 
> RFPOnly is not documented, but looks like is may be what is wanted here?
> 
> Would it be sufficient to use that, if we ensure that sampleRate cannot be
> higher than 96k/s?

The jittering we do provides additional defense against constructing a higher resolution timer using clock edge techniques or clock arbitrage; and given the ease at which one can construct multiple AudioContexts and get them all going at the same time I think that's important.  I think/hope the patch I attached here will address a lot (if not all) of these bugs.
Flags: needinfo?(tom)
Comment on attachment 8964897 [details] [diff] [review]
Skipped on linux and windows

># HG changeset patch
># User Eliza Balazs <ebalazs@mozilla.com>
># Parent  dd31fb345c11732432fa61a7d8a1b727d41b4a1c
>Bug 1446346 - Disable test_audioParamLinearRamp.html on linux and windows for frequent failures. r=jmaher
>
>diff --git a/dom/media/webaudio/test/mochitest.ini b/dom/media/webaudio/test/mochitest.ini
>--- a/dom/media/webaudio/test/mochitest.ini
>+++ b/dom/media/webaudio/test/mochitest.ini
>@@ -69,6 +69,7 @@ tags=capturestream
> [test_audioParamExponentialRamp.html]
> [test_audioParamGain.html]
> [test_audioParamLinearRamp.html]
>+skip-if = os == "linux" || os == "win" #bug 1446346
> [test_audioParamSetCurveAtTime.html]
> [test_audioParamSetCurveAtTimeTwice.html]
> [test_audioParamSetCurveAtTimeZeroDuration.html]
Attachment #8964897 - Attachment is obsolete: true
Keywords: regression
Version: unspecified → 60 Branch
There is little value in applying such a jitter (1ms) that is less than the
typical precision of currentTime (minimum of 2.6ms, but more likely at least
5.4ms), because the jittered time can be rounded back to the pre-jittered
value.  Even if jittering by a larger value, there are plenty of other
techniques that can be used to detect the MSG (audio) processing intervals.

If you are concerned about clock edge techniques, then a couple of options are
available:

1) Jitter a delay on messages from the MSG thread to the main thread.
   Increasing latency either way would be sad, but this may be simpler than 2,
   for an interim solution.

2) Reorder the processing of messages on the main thread, so that an attacker
   can't aim to bracket the MSG message with its own events.  We'd like to have
   a separate event queue for MSG messages anyway as that would reduce their
   latency, and so this is definitely the preferred solution.
Comment on attachment 8967241 [details]
Bug 1446346 Remove skips for webaudio tests that should now be fixed

https://reviewboard.mozilla.org/r/235944/#review241764
Attachment #8967241 - Flags: review?(padenot) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c0900d55df7b
If an AudioContext hasn't been started, do not bother clamping/jittering the CurrentTime r=padenot
https://hg.mozilla.org/integration/autoland/rev/2b5a2f284d0f
Remove skips for webaudio tests that should now be fixed r=padenot
Keywords: checkin-needed
Looks like there was a failure on autoland after this landed :(
https://treeherder.mozilla.org/logviewer.html#?repo=autoland&job_id=173413252
Flags: needinfo?(achronop)
Adding Tom ...
Flags: needinfo?(achronop) → needinfo?(tom)
Going to try to reproduce this with debugging information....
Flags: needinfo?(tom)
https://treeherder.mozilla.org/logviewer.html#?job_id=173628884&repo=mozilla-central&lineNumber=11475

23:13:08     INFO -  171 INFO TEST-START | dom/media/webaudio/test/test_audioParamLinearRamp.html
23:13:08     INFO -  TEST-INFO | started process screenshot
23:13:08     INFO -  TEST-INFO | screenshot: exit 0
23:13:08     INFO -  Buffered messages logged at 23:13:08
23:13:08     INFO -  172 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of channels for expected buffer 0
23:13:08     INFO -  173 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of expected frames
23:13:08     INFO -  Buffered messages finished
23:13:08    ERROR -  174 INFO TEST-UNEXPECTED-FAIL | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0.8999999985098839, first bad index: 0 with test-data offset 0 and expected-data offset 0; corresponding values 1 and 0.10000000149011612 --- differences - got 999, expected +0
23:13:08     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:312:5
23:13:08     INFO -      compareChannels@dom/media/webaudio/test/webaudio.js:78:3
23:13:08     INFO -      compareBuffers@dom/media/webaudio/test/webaudio.js:103:5
23:13:08     INFO -      testOutput/sp.onaudioprocess@dom/media/webaudio/test/webaudio.js:214:11
23:13:08     INFO -      EventHandlerNonNull*testOutput@dom/media/webaudio/test/webaudio.js:211:9
23:13:08     INFO -      runTestOnContext@dom/media/webaudio/test/webaudio.js:202:9
23:13:08     INFO -      testOnNormalContext@dom/media/webaudio/test/webaudio.js:222:7
23:13:08     INFO -      runTestFunction@dom/media/webaudio/test/webaudio.js:253:5
23:13:08     INFO -      rval@SimpleTest/SimpleTest.js:146:17
23:13:08     INFO -      EventHandlerNonNull*this.addLoadEvent@SimpleTest/SimpleTest.js:171:13
23:13:08     INFO -      @SimpleTest/SimpleTest.js:1444:5
23:13:08     INFO -  175 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of channels for expected buffer 0
23:13:08     INFO -  176 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of expected frames
23:13:08     INFO -  177 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of input buffer channels
23:13:08     INFO -  178 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0, first bad index: -1 with test-data offset 0 and expected-data offset 0; corresponding values undefined and undefined --- differences
23:13:08     INFO -  179 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of channels for expected buffer 0
23:13:08     INFO -  180 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of expected frames
23:13:08     INFO -  181 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | Correct number of input buffer channels
23:13:08     INFO -  182 INFO TEST-PASS | dom/media/webaudio/test/test_audioParamLinearRamp.html | maxDifference: 0, first bad index: -1 with test-data offset 0 and expected-data offset 0; corresponding values undefined and undefined --- differences
23:13:08     INFO -  GECKO(5504) | MEMORY STAT | vsize 438MB | vsizeMaxContiguous 720MB | residentFast 77MB | heapAllocated 14MB
23:13:08     INFO -  183 INFO TEST-OK | dom/media/webaudio/test/test_audioParamLinearRamp.html | took 81ms

Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla61 → ---
I got to the bottom of reopened Bug 1446346.  I guess it's not surprising: This is still occurring because we (occasionally) will jitter up a CurrentTime of 0 that _has_ been started. I think karlt's approach is best at this point.
Attachment #8966627 - Attachment is obsolete: true
Attachment #8967241 - Attachment is obsolete: true
Attachment #8968653 - Flags: review?(padenot)
Comment on attachment 8968653 [details] [diff] [review]
Do not jitter if sampleRate is greater than our interval

># HG changeset patch
># User Tom Ritter <tom@mozilla.com>
># Date 1523997351 18000
>#      Tue Apr 17 15:35:51 2018 -0500
># Node ID 9e4d896dd34a201c84397d187a64995f7e1c11de
># Parent  9a37394c21c6a378815f1f9ae1eeefb3eb3db106
>Bug 1446346 Do not clamp or jitter the AudioContext's CurrentTime if its interval is larger than our precision r?padenot
>
>MozReview-Commit-ID: Bc1cto3pBKL
>
>diff --git a/dom/media/webaudio/AudioContext.cpp b/dom/media/webaudio/AudioContext.cpp
>--- a/dom/media/webaudio/AudioContext.cpp
>+++ b/dom/media/webaudio/AudioContext.cpp
>@@ -663,27 +663,32 @@ AudioContext::DestinationStream() const
>   return nullptr;
> }
>
> double
> AudioContext::CurrentTime()
> {
>   MediaStream* stream = Destination()->Stream();
>
>-  if (!mIsStarted &&
>-    stream->StreamTimeToSeconds(stream->GetCurrentTime()) == 0) {
>-      return 0;
>+  double rawTime = stream->StreamTimeToSeconds(stream->GetCurrentTime());
>+
>+  // CurrentTime increments in intervals of 128/sampleRate. If the Timer
>+  // Precision Reduction is smaller than this interval, the jittered time
>+  // can always be reversed to the raw step of the interval. In that case
>+  // we can simply return the un-reduced time; and avoid breaking tests.
>+  // We have to convert each variable into a common magnitude, we choose ms.
>+  if ((128/mSampleRate) * 1000.0 > nsRFPService::TimerResolution() / 1000.0) {
>+    return rawTime;
>   }
>
>   // The value of a MediaStream's CurrentTime will always advance forward; it will never
>   // reset (even if one rewinds a video.) Therefore we can use a single Random Seed
>   // initialized at the same time as the object.
>   return nsRFPService::ReduceTimePrecisionAsSecs(
>-    stream->StreamTimeToSeconds(stream->GetCurrentTime()),
>-    GetRandomTimelineSeed());
>+    rawTime, GetRandomTimelineSeed());
> }
>
> void AudioContext::DisconnectFromOwner()
> {
>   mIsDisconnecting = true;
>   Shutdown();
>   DOMEventTargetHelper::DisconnectFromOwner();
> }
>diff --git a/toolkit/components/resistfingerprinting/nsRFPService.cpp b/toolkit/components/resistfingerprinting/nsRFPService.cpp
>--- a/toolkit/components/resistfingerprinting/nsRFPService.cpp
>+++ b/toolkit/components/resistfingerprinting/nsRFPService.cpp
>@@ -102,18 +102,19 @@ nsRFPService::GetOrCreate()
>
>     ClearOnShutdown(&sRFPService);
>     sInitialized = true;
>   }
>
>   return sRFPService;
> }
>
>-inline double
>-TimerResolution()
>+/* static */
>+double
>+nsRFPService::TimerResolution()
> {
>   if(nsRFPService::IsResistFingerprintingEnabled()) {
>     return max(100000.0, (double)sResolutionUSec);
>   }
>   return sResolutionUSec;
> }
>
> /* static */
>diff --git a/toolkit/components/resistfingerprinting/nsRFPService.h b/toolkit/components/resistfingerprinting/nsRFPService.h
>--- a/toolkit/components/resistfingerprinting/nsRFPService.h
>+++ b/toolkit/components/resistfingerprinting/nsRFPService.h
>@@ -157,16 +157,17 @@ class nsRFPService final : public nsIObs
> {
> public:
>   NS_DECL_ISUPPORTS
>   NS_DECL_NSIOBSERVER
>
>   static nsRFPService* GetOrCreate();
>   static bool IsResistFingerprintingEnabled();
>   static bool IsTimerPrecisionReductionEnabled(TimerPrecisionType aType);
>+  static double TimerResolution();
>
>   enum TimeScale {
>     Seconds      = 1,
>     MilliSeconds = 1000,
>     MicroSeconds = 1000000
>   };
>
>   // The following Reduce methods can be called off main thread.
Attachment #8968653 - Attachment is patch: true
Comment on attachment 8968653 [details] [diff] [review]
Do not jitter if sampleRate is greater than our interval

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

Thanks you for this!

::: dom/media/webaudio/AudioContext.cpp
@@ +667,5 @@
>  {
>    MediaStream* stream = Destination()->Stream();
> +  double rawTime = stream->StreamTimeToSeconds(stream->GetCurrentTime());
> +
> +  // CurrentTime increments in intervals of 128/sampleRate. If the Timer

This is true at a spec level. In practice, we update in multiples of 128/sampleRate. With builds we ship (with default prefs), the finest resolution we have is on macbook pros, where it's 256.

However, if this is acceptable to you, I think keeping it at 128/sampleRate is good, one less thing to think about when we align our implementation with the spec.
Attachment #8968653 - Flags: review?(padenot) → review+
Assignee: achronop → tom
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/088d40fae53d
Do not clamp or jitter the AudioContext's CurrentTime if its interval is larger than our precision. r=padenot
Keywords: checkin-needed
Attachment #8968653 - Attachment is obsolete: true
Flags: needinfo?(tom)
Attachment #8969701 - Flags: review?(padenot)
Attachment #8969701 - Flags: review?(padenot) → review+
Comment on attachment 8970207 [details] [diff] [review]
Do not jitter if sampleRate is greater than our interval

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

using includes() instead of indexOf()
Attachment #8970207 - Flags: review+
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8981a88a2360
Do not clamp or jitter the AudioContext's CurrentTime if its interval is larger than our precision. r=jib
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8981a88a2360
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(In reply to Paul Adenot (:padenot) from comment #26)
> roc contacted me by email with a Pernosco link for this one, which allowed
> me to quickly find the cause for this issue, but it was unclear to me how
> public Persnoco is at the minute so I didn't mention anything.

It's not super secret, say whatever you need to say :-).
Please request Beta approval on this when you're comfortable doing so.
Flags: needinfo?(tom)
Comment on attachment 8970207 [details] [diff] [review]
Do not jitter if sampleRate is greater than our interval

Approval Request Comment
[Feature/Bug causing the regression]: Test Intermittents

[User impact if declined]: None. 

[Is this code covered by automated tests?]: Yes

[Has the fix been verified in Nightly?]: Yes.

[Needs manual test from QE? If yes, steps to reproduce]:  No

[List of other uplifts needed for the feature/fix]: None

[Is the change risky?]: No

[Why is the change risky/not risky?]: We're relaxing time precision rounding for AudioContext, which is not an invasive change.

[String changes made/needed]: No
Flags: needinfo?(tom)
Attachment #8970207 - Flags: approval-mozilla-beta?
Comment on attachment 8970207 [details] [diff] [review]
Do not jitter if sampleRate is greater than our interval

While we do want to fix intermittent test failures promptly, this is the last beta build and I'd prefer to let this one ride the 61 train.
Attachment #8970207 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
I'd like to keep this on the radar for 60.1 consideration during the next cycle after it's had some more bake time.
Comment on attachment 8970207 [details] [diff] [review]
Do not jitter if sampleRate is greater than our interval

Taking this now for ESR60 that it's had time to bake.
Attachment #8970207 - Flags: approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.