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

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[Karl Tomlinson (:karlt)]

+++ 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.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: 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.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: 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

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: 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.)

Comment 4

[Paul Adenot (: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

Comment 5

[Paul Adenot (:padenot)]

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 6

[Paul Adenot (:padenot)]

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

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea6714b89f5c
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff4b05caac7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4cf5dc1ab2c

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ea6714b89f5c
https://hg.mozilla.org/mozilla-central/rev/ff4b05caac7f
https://hg.mozilla.org/mozilla-central/rev/e4cf5dc1ab2c