AudioBufferSourceNode and OscillatorNode don't dispatch ended event when OfflineAudioContext completes soon after source end

RESOLVED FIXED in Firefox 45

Status

()

Core
Web Audio
P2
normal
Rank:
25
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
+++ This bug was initially created as a clone of Bug #914016 +++

See attachment 807068 [details].

Expected results:
ended
complete

Actual results:
complete

OscillatorNodeEngine doesn't set aFinished until the block after it finishes.


See also the comment in bug content/media/test/crashtests/offline-buffer-source-ended-1.html

AudioNodeStream::ProduceOutput() doesn't mark the stream as finished until the block after aFinished is set, which affects both OscillatorNode and AudioBufferSourceNode.

onended could be implemented using DispatchToMainThreadAfterStreamStateUpdate and AddListener instead of AddMainThreadListener, and main thread updates would not be required.

See also bug 923301, because downstream nodes depend on playing references not being released until strictly *after* the last block with any sound is produced.
Priority: -- → P2
(Assignee)

Updated

3 years ago
Assignee: nobody → karlt
Depends on: 1217625
(Assignee)

Comment 1

3 years ago
Created attachment 8677829 [details]
MozReview Request: bug 930257 finish BufferSource on processing last non-null block r?padenot

bug 930257 finish BufferSource on processing last non-null block r?padenot

Since changes for bug 1217625, the node and downstream nodes won't be made
inactive until after downstream nodes have done their processing, and so there
is no need to wait for the first silent output block.

This essentially reverts 5c607f3f39d55544838f3111ede9e11a00d3c25e.
Attachment #8677829 - Flags: review?(padenot)
(Assignee)

Comment 2

3 years ago
Created attachment 8677830 [details]
MozReview Request: bug 930257 finish Oscillator on processing last non-null block r?padenot

bug 930257 finish Oscillator on processing last non-null block r?padenot
Attachment #8677830 - Flags: review?(padenot)
(Assignee)

Comment 3

3 years ago
Created attachment 8677831 [details]
MozReview Request: bug 930257 schedule Analyser inactive check when sending last null chunk r?padenot

bug 930257 schedule Analyser inactive check when sending last null chunk r?padenot

(Doing the extra ProcessBlock for the sake of downstream nodes was unnecessary
 even before the inactive check was delayed until after their processing, because
 downstream nodes would have only had null chunks to process anyway.)
Attachment #8677831 - Flags: review?(padenot)
Comment on attachment 8677829 [details]
MozReview Request: bug 930257 finish BufferSource on processing last non-null block r?padenot

https://reviewboard.mozilla.org/r/23025/#review20587
Attachment #8677829 - Flags: review?(padenot) → review+

Updated

3 years ago
Attachment #8677830 - Flags: review?(padenot) → review+
Comment on attachment 8677830 [details]
MozReview Request: bug 930257 finish Oscillator on processing last non-null block r?padenot

https://reviewboard.mozilla.org/r/23027/#review20589
Comment on attachment 8677831 [details]
MozReview Request: bug 930257 schedule Analyser inactive check when sending last null chunk r?padenot

https://reviewboard.mozilla.org/r/23029/#review20591
Attachment #8677831 - Flags: review?(padenot) → review+

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ea6714b89f5c
https://hg.mozilla.org/mozilla-central/rev/ff4b05caac7f
https://hg.mozilla.org/mozilla-central/rev/e4cf5dc1ab2c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.