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.

Sign up for GitHub

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

Jump to bottom

Remove or rename 'volume' #585

Closed
armax00 opened this issue May 9, 2019 · 12 comments · Fixed by #588
Closed

Remove or rename 'volume' #585

armax00 opened this issue May 9, 2019 · 12 comments · Fixed by #588
Assignees
@armax00
Labels
PR exists

Comments

@armax00
Copy link
Collaborator

armax00 commented May 9, 2019

The 'volume' constrainable property is defined as being a multiplier applied to the linear audio samples. This means that this property does not control the volume of the audio device capture (which can be adjusted by the user and determined by the OS/HW) but rather the gain. More precisely, since the range allowed is between 0 and 1, this constrainable property controls the loss (gain between 0 and 1).

I propose that this property should be renamed from volume to either gain or loss.

Further, the description of volume in https://www.w3.org/TR/mediacapture-streams/#def-constraint-volume should be clearer in identifying it as gain/loss.
@alvestrand
Copy link
Contributor

alvestrand commented May 9, 2019 •
edited

Seems to not be implemented in Safari or Firefox at the moment.
Generally I prefer to avoid renames - but improving the description (and adding tests) is a Good Thing.

@guidou
Copy link
Contributor

guidou commented May 9, 2019 •
edited

My interpretation of the spec is not that it is a multiplier applied to the samples, even though the word multiplier is used. I interpret that the value of the property is a value between 0 to 1 such that the value, when multiplied by a constant (or maybe a function) results in the actual volume, with 1 yielding the maximum value and 0 the minimum.
This said, I would instead propose to remove this from the spec. No browser really implements it (although Chrome reports a volume setting for some tracks), and there seems to be little demand for it.

@armax00
Copy link
Collaborator Author

armax00 commented May 10, 2019

Based on a chat with alvestrand, it was clarified that this constrainable property was meant to determine loss, more precisely an additional layer of processing on a track to lower the volume of the track. I am not sure about the initial intention and use-case for such a constraint, but it looks to me the constrainable property should either be removed or renamed.

@henbos
Copy link
Contributor

henbos commented May 15, 2019

Shall we remove it? ;)

@henbos henbos changed the title 'volume' constrainable property requires a better name. Remove or rename 'volume' May 15, 2019
@guidou
Copy link
Contributor

guidou commented May 15, 2019

+1 to remove it

@eric-carlson
Copy link

WebKit implements 'volume', but we won't object to removing it.

@alvestrand
Copy link
Contributor

seems that only Safari has implemented it, and don't object to removing it.
Marking "ready for PR".

@armax00 armax00 mentioned this issue May 16, 2019
Merged
@armax00
Copy link
Collaborator Author

armax00 commented May 20, 2019

Anything missing on the PR/bug?

@henbos henbos assigned armax00 and unassigned alvestrand May 24, 2019
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 27, 2019
@chromium-wpt-export-bot
[MediaStreamTrack] Remove tests and existing support for the 'volume'…
1543f8f 
… property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
TODO(armax): file an i2d&r.

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 28, 2019
@chromium-wpt-export-bot
[MediaStreamTrack] Remove tests and existing support for the 'volume'…
1d74d98 
… property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
TODO(armax): file an i2d&r.

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 28, 2019
@chromium-wpt-export-bot
[MediaStreamTrack] Remove tests and existing support for the 'volume'…
e233d0f 
… property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
@foolip
Copy link
Member

foolip commented May 28, 2019

@eric-carlson is there a WebKit bug tracking the removal of this? There's an Intent to Deprecate and Remove: volume property on blink-dev now, which ought to leave WebKit as the only engine supporting it.

@eric-carlson
Copy link

@eric-carlson is there a WebKit bug tracking the removal of this? There's an Intent to Deprecate and Remove: volume property on blink-dev now, which ought to leave WebKit as the only engine supporting it.

@foolip yes, I filed [MediaStream] remove 'volume' constraint

@foolip
Copy link
Member

foolip commented May 28, 2019

Thanks Eric!

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 29, 2019
@chromium-wpt-export-bot
[MediaStreamTrack] Remove tests and existing support for the 'volume'…
00fb4fa 
… property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Armando Miraglia <armax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664126}
Hexcles pushed a commit to web-platform-tests/wpt that referenced this issue May 31, 2019
@Hexcles
[MediaStreamTrack] Remove tests and existing support for the 'volume'…
e951b47 
… property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Armando Miraglia <armax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664126}
@saschanaz saschanaz mentioned this issue Jun 10, 2019
Merged
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 19, 2019
@moz-wptsync-bot @jgraham
Bug 1554618 [wpt PR 17028] - [MediaStreamTrack] Remove tests and exis…
499b502 
…ting support for the 'volume' property., a=testonly

Automatic update from web-platform-tests
[MediaStreamTrack] Remove tests and existing support for the 'volume' property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Armando Miraglia <armax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664126}

--

wp5At-commits: e951b47b4c107995e7696ff9db7b2b0bafcde56b
wpt-pr: 17028
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jun 19, 2019
@moz-wptsync-bot @jgraham
Bug 1554618 [wpt PR 17028] - [MediaStreamTrack] Remove tests and exis…
4e4ed86 
…ting support for the 'volume' property., a=testonly

Automatic update from web-platform-tests
[MediaStreamTrack] Remove tests and existing support for the 'volume' property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Armando Miraglia <armax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664126}

--

wp5At-commits: e951b47b4c107995e7696ff9db7b2b0bafcde56b
wpt-pr: 17028
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Jul 23, 2019
[MediaStreamTrack] Remove tests and existing support for the 'volume'…
79ab9d6 
… property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Armando Miraglia <armax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664126}
@pavelserbajlo
Copy link

I understand the intent here, but removal of this has caused a regression for us as we'd rely on this in order to normalize our comparative audio pipeline processing in Chrome. Since decision has been made to remove this, what are the other ways to read the microphone input gain for the developers?

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
@marco-c
Bug 1554618 [wpt PR 17028] - [MediaStreamTrack] Remove tests and exis…
bdc98ee 
…ting support for the 'volume' property., a=testonly

Automatic update from web-platform-tests
[MediaStreamTrack] Remove tests and existing support for the 'volume' property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidouchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Commit-Queue: Armando Miraglia <armaxchromium.org>
Cr-Commit-Position: refs/heads/master{#664126}

--

wp5At-commits: e951b47b4c107995e7696ff9db7b2b0bafcde56b
wpt-pr: 17028

UltraBlame original commit: e092aed11b894bab9fe18ffd6b8910ddcdd220fc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
@marco-c
Bug 1554618 [wpt PR 17028] - [MediaStreamTrack] Remove tests and exis…
81f1f8b 
…ting support for the 'volume' property., a=testonly

Automatic update from web-platform-tests
[MediaStreamTrack] Remove tests and existing support for the 'volume' property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidouchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Commit-Queue: Armando Miraglia <armaxchromium.org>
Cr-Commit-Position: refs/heads/master{#664126}

--

wp5At-commits: e951b47b4c107995e7696ff9db7b2b0bafcde56b
wpt-pr: 17028

UltraBlame original commit: e092aed11b894bab9fe18ffd6b8910ddcdd220fc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
@marco-c
Bug 1554618 [wpt PR 17028] - [MediaStreamTrack] Remove tests and exis…
f7fb350 
…ting support for the 'volume' property., a=testonly

Automatic update from web-platform-tests
[MediaStreamTrack] Remove tests and existing support for the 'volume' property.

Since 'volume' has been removed from the standard specification[1], this
CL removes support for the 'volume' from the output of
MediaStreamTrack.getSettings(). Further, this CL removes the related
existing tests.

Intent to deprecate and remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/8-Qn7pu41Kk

[1] w3c/mediacapture-main#585

BUG=942016

Change-Id: I8cfcd5d119f79d4da73ee5aa94770f2cd2b7f881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630156
Reviewed-by: Guido Urdaneta <guidouchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Commit-Queue: Armando Miraglia <armaxchromium.org>
Cr-Commit-Position: refs/heads/master{#664126}

--

wp5At-commits: e951b47b4c107995e7696ff9db7b2b0bafcde56b
wpt-pr: 17028

UltraBlame original commit: e092aed11b894bab9fe18ffd6b8910ddcdd220fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@armax00 armax00

Labels
PR exists
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove the 'volume' constraint.
7 participants
@alvestrand @foolip @armax00 @eric-carlson @pavelserbajlo @henbos @guidou