Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #12 by adding outputTimeStamp attribute to AudioDestinationNode #754

Merged
merged 1 commit into from
May 12, 2016

Conversation

pozdnyakov
Copy link

@pozdnyakov pozdnyakov commented Mar 8, 2016

This patch introduces 'AudioDestinationNode.outputTimeStamp'
and 'AudioDestinationNode.outpuLatency' attributes, so that it's possible
to estimate when the given context's stream position is audible.

</p>
</dd>
<dt>
AudioTimestamp&lt;void&gt; getPosition()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getPosition seems rather vague. Is there a better name?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would getStreamPosition be better?

pozdnyakov pushed a commit to pozdnyakov/chromium-crosswalk that referenced this pull request Mar 23, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
pozdnyakov pushed a commit to pozdnyakov/chromium-crosswalk that referenced this pull request Mar 24, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
pozdnyakov pushed a commit to pozdnyakov/chromium-crosswalk that referenced this pull request Mar 31, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
pozdnyakov pushed a commit to pozdnyakov/chromium-crosswalk that referenced this pull request Mar 31, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
pozdnyakov pushed a commit to pozdnyakov/chromium-crosswalk that referenced this pull request Apr 1, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
rakuco pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Apr 8, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Apr 8, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
@pozdnyakov pozdnyakov changed the title Fix #12 by by adding getPosition() method to AudioDestinationNode Fix #12 by by adding devicePosition attribute to AudioDestinationNode Apr 12, 2016
@pozdnyakov pozdnyakov changed the title Fix #12 by by adding devicePosition attribute to AudioDestinationNode Fix #12 by adding devicePosition attribute to AudioDestinationNode Apr 12, 2016
</p>
<p>
The <a><code>latency</code></a> attribute value depends on the
platform and it does not change for the context's lifetime.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you unplug your usb device ? This changes the sound card and probably changes the latency as well. Same think if you disconnect an bluetooth device that is using a2ps, you go from 400+ms to low latency.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attribute value should change in this case indeed, I'll update the description accordingly.. Thanks for pointing it out.

mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Apr 18, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
@pozdnyakov
Copy link
Author

@padenot does it look OK now?

@@ -2309,6 +2309,112 @@ <h2 id="AudioDestinationNode">
not be changed.
</p>
</dd>
<dt>
readonly attribute double latency
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we also now have baseLatency (for the playback category stuff), latency is, perhaps, too generic. Maybe outputLatency, or deviceOutputLatency, or something else?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have the baseLatency in the context, this needs to be moved to there. Getting two different latency values from different entities is weird. Also this should be renamed - I prefer deviceOutputLatency.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer outputLatency as it matches to outputTimeStamp in the latest patch. IMO outputLatency should remain at AudioDestinationNode as this interface is representing an output device, so it's a natural place to contain the attribute representing output device latency. wdyt?

…'AudioDestinationNode.outputLatency'

This patch introduces 'AudioDestinationNode.outputTimeStamp' and 'AudioDestinationNode.outputLatency' attributes, so that it's possible to estimate when the given context's stream position is audible.
@pozdnyakov
Copy link
Author

All checks have passed successfully, @rtoy could it be merged or we're still waiting for some input?

@rtoy
Copy link
Member

rtoy commented May 11, 2016

I would still like someone else to review it too. Still lgtm from me of
course.

On Wed, May 11, 2016 at 1:25 AM, Mikhail Pozdnyakov <
notifications@github.com> wrote:

All checks have passed successfully, @rtoy https://github.com/rtoy
could it be merged or we're still waiting for some input?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#754 (comment)

Ray

@padenot
Copy link
Member

padenot commented May 12, 2016

I just re-read everything, I think the text is good now.

@padenot padenot merged commit 1c6778b into WebAudio:gh-pages May 12, 2016
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request May 13, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request May 13, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Jun 11, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Jun 15, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Jul 19, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Aug 1, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Aug 1, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
msisov pushed a commit to msisov/chromium-crosswalk that referenced this pull request Aug 16, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
msisov pushed a commit to msisov/chromium-crosswalk that referenced this pull request Aug 16, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Aug 26, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
msisov pushed a commit to msisov/chromium-crosswalk that referenced this pull request Aug 31, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Sep 1, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
mrunalk pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Sep 1, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
rakuco pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Sep 21, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
rakuco pushed a commit to crosswalk-project/chromium-crosswalk that referenced this pull request Sep 30, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
rakuco added a commit to rakuco/chromium-crosswalk that referenced this pull request Oct 5, 2016
This reverts the following commits:
* 2b2cbc8 ("[Windows] Implementation of 'AudioDestinationNode.devicePosition' attribute")
* 43d41bf ("Fix calling of AudioOutputStream::AudioSourceCallback API")
* fd8c95a ("Remove unused private field device_position_")

This code was added when Crosswalk was being used as an alternative
implementation for some changes to the WebAudio spec:
* WebAudio/web-audio-api#12
* WebAudio/web-audio-api#754

The spec has since been changed and upstream Chromium is being adapted,
so there is no need to keep these patches in Crosswalk any longer.

Additionally, this code in its present form has caused serious
regressions in WebRTC support (the WebRTC Crosswalk sample did not
produce any sound).

Thanks to Michael Walton <mike@farsouthnet.com> for the investigation
leading to this commit.

BUG=XWALK-7030
BUG=XWALK-7225
rakuco added a commit to rakuco/chromium-crosswalk that referenced this pull request Oct 5, 2016
This reverts the following commits in the crosswalk-22/52.0.2743.116
branch:
* 2b2cbc8 ("[Windows] Implementation of 'AudioDestinationNode.devicePosition' attribute")
* 43d41bf ("Fix calling of AudioOutputStream::AudioSourceCallback API")

This code was added when Crosswalk was being used as an alternative
implementation for some changes to the WebAudio spec:
* WebAudio/web-audio-api#12
* WebAudio/web-audio-api#754

The spec has since been changed and upstream Chromium is being adapted,
so there is no need to keep these patches in Crosswalk any longer.

Additionally, this code in its present form has caused serious
regressions in WebRTC support (the WebRTC Crosswalk sample did not
produce any sound).

Thanks to Michael Walton (mike@farsouthnet.com) for the investigation
leading to this commit.

BUG=XWALK-7030
BUG=XWALK-7226

(cherry picked from commit 7807ac7)
rakuco added a commit to rakuco/chromium-crosswalk that referenced this pull request Oct 5, 2016
This reverts the following commits in the crosswalk-21/51.0.2704.106
branch:
* e230a89 ("[Windows] Implementation of 'AudioDestinationNode.devicePosition' attribute")
* a0388f7 ("Fix calling of AudioOutputStream::AudioSourceCallback API")

This code was added when Crosswalk was being used as an alternative
implementation for some changes to the WebAudio spec:
* WebAudio/web-audio-api#12
* WebAudio/web-audio-api#754

The spec has since been changed and upstream Chromium is being adapted,
so there is no need to keep these patches in Crosswalk any longer.

Additionally, this code in its present form has caused serious
regressions in WebRTC support (the WebRTC Crosswalk sample did not
produce any sound).

Thanks to Michael Walton (mike@farsouthnet.com) for the investigation
leading to this commit.

BUG=XWALK-7030
BUG=XWALK-7226

(cherry picked from commit 7807ac7)
huningxin pushed a commit to huningxin/chromium-croswalk that referenced this pull request Oct 9, 2016
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants