1618225 - In AudioBufferSourceNode when loop is true, duration is not taken into account

Status: RESOLVED FIXED

(Core :: Web Audio, enhancement, P3)

Description

[Andrés Souto]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.116 Safari/537.36

Steps to reproduce:

https://jsfiddle.net/kai670/h1ex5ky8/7/

Actual results:

The sound is played in an infinite loop that never ends.

Expected results:

The sound must be played in loop during 5 seconds.

That is the behaviour in Chrome and what I understand reading the spec:

The duration parameter describes the duration of sound to be played, expressed as seconds of total buffer content to be output, including any whole or partial loop iterations. The units of duration are independent of the effects of playbackRate. For example, a duration of 5 seconds with a playback rate of 0.5 will output 5 seconds of buffer content at half speed, producing 10 seconds of audible output. A RangeError exception MUST be thrown if duration is negative.

https://www.w3.org/TR/webaudio/

Comment 1

[Karl Tomlinson (:karlt)]

The specified behavior changed for https://github.com/WebAudio/web-audio-api/issues/421.
The regression from that has also been resolved now.

Comment 2

[carnould]

Attachment: Bug 1618225 - Set stop time when using loop with duration. r=padenot

This sets the value of stop in all the cases.

Comment 3

[carnould]

Attachment: Bug 1618225 - Set stop time when using loop with duration. r=padenot

The AudioBufferSourceNode can now be looping and stop after duration.

Comment 4

[carnould]

Attachment: Bug 1618225 - Updated mochitest for loop with duration r=padenot

Depends on D79873

Comment 5

[carnould]

Attachment: Bug 1618225 - Added wpt test for loop with duration r=padenot

Depends on D79874

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f74c10d88304
Set stop time when using loop with duration. r=padenot
https://hg.mozilla.org/integration/autoland/rev/99ac03bfe9ce
Updated mochitest for loop with duration r=padenot
https://hg.mozilla.org/integration/autoland/rev/feff7b5ed5b1
Added wpt test for loop with duration r=padenot

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24326 for changes under testing/web-platform/tests

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/24326
* Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/GQQiYe2DQTq0PVaNtvHwSw)

Comment 9

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/f74c10d88304
https://hg.mozilla.org/mozilla-central/rev/99ac03bfe9ce
https://hg.mozilla.org/mozilla-central/rev/feff7b5ed5b1

Comment 10

[carnould]

Reopening because of the above regression.

Comment 11

[Julien Cristau [:jcristau]]

Backed out from trunk and beta in bug 1650030.

Comment 12

[carnould]

Attachment: Bug 1618225 - Fixed loop with duration. r?padenot

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by jgraham

Comment 14

[carnould]

Attachment: Bug 1618225 - Changed mBufferEnd to mRemainingFrame for loop to stop. r?padenot

Comment 15

[carnould]

Attachment: Bug 1618225 - Added DURATION to AudioBufferSourceNode. r?karlt

It allows simpler handling of duration, and it works on looping sources.
Also removed now unnecessary calculations ont mBufferEnd to handle duration.

Comment 16

[carnould]

Attachment: Bug 1618225 - Update mochitest. r?karlt

Depends on D84402

Comment 17

[carnould]

Attachment: Bug 1618225 - Added wpt. r?karlt

Depends on D84403

Comment 18

[carnould]

Attachment: Bug 1618225 - Use llround function instead of + 0.5 r?karlt

Let me know if you want to change NS_lround() function call too.

Depends on D84481

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:koalab1999, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 20

[Karl Tomlinson (:karlt)]

I've added some test jobs to https://treeherder.mozilla.org/#/jobs?repo=try&revision=92a62d9bfad13e8a17b8377e9385ee0d1a97ed30.
If those complete successfully (or with only unrelated failures) please add the "Check-in Needed" project "Tag" to https://phabricator.services.mozilla.com/differential/revision/edit/85583/

Comment 21

[Paul Adenot (:padenot)]

They don't, it's probably not too hard to fix them.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bfda8cd8ce2f
Added DURATION to AudioBufferSourceNode. r=karlt
https://hg.mozilla.org/integration/autoland/rev/3aa921cab417
Update mochitest. r=padenot
https://hg.mozilla.org/integration/autoland/rev/97ecf4557cf4
Added wpt. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0c7b5d5a2307
Use llround function instead of + 0.5 r=karlt

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/26315 for changes under testing/web-platform/tests

Comment 24

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/bfda8cd8ce2f
https://hg.mozilla.org/mozilla-central/rev/3aa921cab417
https://hg.mozilla.org/mozilla-central/rev/97ecf4557cf4
https://hg.mozilla.org/mozilla-central/rev/0c7b5d5a2307