1501709 - AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation

Status: RESOLVED FIXED

(Core :: Web Audio, enhancement, P2)

Description

[Arnaud Bienner]

Follow up of bug 1473467.
This bug is about implementing the last step of registerProcessor:
"Queue a task to the control thread to add the key-value pair (name - descriptors) to the node name to parameter descriptor map of the associated BaseAudioContext."

Comment 1

[Arnaud Bienner]

As discussed with Karl on IRC, AudioWorkletImpl should keep a reference on the stream of the audioContext used to create the AudioWorkletImpl.
From there, we could access the AudioContext (through stream->Engine()->NodeMainThread()->Context()).
Since WorkletImpl objects can be addressed from both threads, that would be the way to have communication between main/control thread and worklet thread.

This will also be useful to implement AudioWokrletGlobalScope::currentTime/Frame etc. later.

Comment 2

[Arnaud Bienner]

Attachment: Bug 1501709 - AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation. r=karlt

Comment 3

[Arnaud Bienner]

Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=516861db07d8040fddaa77824560b102c2f64e1b&selectedJob=214637720

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e1dbfd10378
AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation. r=karlt,baku,froydnj

Comment 5

[Razvan Maries]

Backed out for build bustages on Windows platform, CLOSED TREE.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0e1dbfd1037861318aa39868eb658cc26ed225fc

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=214769868&repo=autoland&lineNumber=21301

04:34:50     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.8.4/VC/bin/Hostx64/x86/cl.exe -FoTestCodeGenBinding.obj -c  -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/dom/bindings/test -Iz:/build/build/src/obj-firefox/dom/bindings/test -Iz:/build/build/src/obj-firefox/dist/include/mozilla/dom -Iz:/build/build/src/obj-firefox/dom/bindings -Iz:/build/build/src/dom/bindings -Iz:/build/build/src/js/xpconnect/src -Iz:/build/build/src/js/xpconnect/wrappers -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -wd4065 -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -O1 -Oi -Oy- -WX  -deps.deps/TestCodeGenBinding.obj.pp    z:/build/build/src/obj-firefox/dom/bindings/TestCodeGenBinding.cpp
04:34:50     INFO -  TestCodeGenBinding.cpp
04:34:50     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/bindings/test'
04:34:50     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:50     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:50     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:50     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:50     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/webgpu'
04:34:50     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.8.4/VC/bin/Hostx64/x86/cl.exe -FoUnified_cpp_dom_webgpu1.obj -c  -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/dom/webgpu -Iz:/build/build/src/obj-firefox/dom/webgpu -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -wd4065 -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -O1 -Oi -Oy- -WX  -deps.deps/Unified_cpp_dom_webgpu1.obj.pp    z:/build/build/src/obj-firefox/dom/webgpu/Unified_cpp_dom_webgpu1.cpp
04:34:50     INFO -  Unified_cpp_dom_webgpu1.cpp
04:34:50     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/webgpu'
04:34:50     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/bindings'
04:34:50     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/bindings'
04:34:50     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/bindings'
04:34:50     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/bindings'
04:34:51     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/bindings'
04:34:51     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.8.4/VC/bin/Hostx64/x86/cl.exe -FoUnifiedBindings0.obj -c  -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DWIN32_LEAN_AND_MEAN -D_WIN32 -DWIN32 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DOS_WIN=1 -D_UNICODE -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DUNICODE -D_WINDOWS -D_SECURE_ATL -DCOMPILER_MSVC -DHAVE_SIDEBAR -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/dom/bindings -Iz:/build/build/src/obj-firefox/dom/bindings -Iz:/build/build/src/obj-firefox/dist/include/mozilla/dom -Iz:/build/build/src/dom/base -Iz:/build/build/src/dom/battery -Iz:/build/build/src/dom/canvas -Iz:/build/build/src/dom/geolocation -Iz:/build/build/src/dom/html -Iz:/build/build/src/dom/indexedDB -Iz:/build/build/src/dom/media/webaudio -Iz:/build/build/src/dom/media/webspeech/recognition -Iz:/build/build/src/dom/svg -Iz:/build/build/src/dom/xbl -Iz:/build/build/src/dom/xml -Iz:/build/build/src/dom/xslt/base -Iz:/build/build/src/dom/xslt/xpath -Iz:/build/build/src/dom/xul -Iz:/build/build/src/js/xpconnect/src -Iz:/build/build/src/js/xpconnect/wrappers -Iz:/build/build/src/layout/generic -Iz:/build/build/src/layout/style -Iz:/build/build/src/layout/xul/tree -Iz:/build/build/src/media/mtransport -Iz:/build/build/src/media/webrtc -Iz:/build/build/src/media/webrtc/signaling/src/common/time_profiling -Iz:/build/build/src/media/webrtc/signaling/src/peerconnection -Iz:/build/build/src/media/webrtc/trunk -Iz:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -Iz:/build/build/src/ipc/chromium/src -Iz:/build/build/src/ipc/glue -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -wd4065 -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -O1 -Oi -Oy- -WX -bigobj  -deps.deps/UnifiedBindings0.obj.pp    z:/build/build/src/obj-firefox/dom/bindings/UnifiedBindings0.cpp
04:34:51     INFO -  UnifiedBindings0.cpp
04:34:51     INFO -  z:\build\build\src\obj-firefox\dom\bindings\AudioParamDescriptorBinding.cpp(105): error C2177: constant too big
04:34:51     INFO -  z:\build\build\src\obj-firefox\dom\bindings\AudioParamDescriptorBinding.cpp(122): error C2177: constant too big
04:34:51     INFO -  z:/build/build/src/config/rules.mk:1131: recipe for target 'UnifiedBindings0.obj' failed
04:34:51     INFO -  mozmake.EXE[4]: *** [UnifiedBindings0.obj] Error 2
04:34:51     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/bindings'
04:34:51     INFO -  mozmake.EXE[4]: *** Waiting for unfinished jobs....
04:34:51     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:51     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:51     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:51     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:52     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:52     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.8.4/VC/bin/Hostx64/x86/cl.exe -FoGrBlurredEdgeFragmentProcessor.obj -c  -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DUNICODE -D_UNICODE -DSKIA_IMPLEMENTATION=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/gfx/skia -Iz:/build/build/src/obj-firefox/gfx/skia -Iz:/build/build/src/gfx/skia/skia/include/c -Iz:/build/build/src/gfx/skia/skia/include/codec -Iz:/build/build/src/gfx/skia/skia/include/config -Iz:/build/build/src/gfx/skia/skia/include/core -Iz:/build/build/src/gfx/skia/skia/include/docs -Iz:/build/build/src/gfx/skia/skia/include/effects -Iz:/build/build/src/gfx/skia/skia/include/encode -Iz:/build/build/src/gfx/skia/skia/include/gpu -Iz:/build/build/src/gfx/skia/skia/include/pathops -Iz:/build/build/src/gfx/skia/skia/include/ports -Iz:/build/build/src/gfx/skia/skia/include/private -Iz:/build/build/src/gfx/skia/skia/include/utils -Iz:/build/build/src/gfx/skia/skia/include/utils/mac -Iz:/build/build/src/gfx/skia/skia/src/codec -Iz:/build/build/src/gfx/skia/skia/src/core -Iz:/build/build/src/gfx/skia/skia/src/gpu -Iz:/build/build/src/gfx/skia/skia/src/gpu/effects -Iz:/build/build/src/gfx/skia/skia/src/gpu/gl -Iz:/build/build/src/gfx/skia/skia/src/gpu/glsl -Iz:/build/build/src/gfx/skia/skia/src/image -Iz:/build/build/src/gfx/skia/skia/src/lazy -Iz:/build/build/src/gfx/skia/skia/src/opts -Iz:/build/build/src/gfx/skia/skia/src/sfnt -Iz:/build/build/src/gfx/skia/skia/src/shaders -Iz:/build/build/src/gfx/skia/skia/src/shaders/gradients -Iz:/build/build/src/gfx/skia/skia/src/sksl -Iz:/build/build/src/gfx/skia/skia/src/utils -Iz:/build/build/src/gfx/skia/skia/src/utils/mac -Iz:/build/build/src/gfx/skia/skia/src/utils/win -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -wd4065 -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -O1 -Oi -Oy-  -deps.deps/GrBlurredEdgeFragmentProcessor.obj.pp    z:/build/build/src/gfx/skia/skia/src/gpu/effects/GrBlurredEdgeFragmentProcessor.cpp
04:34:52     INFO -  GrBlurredEdgeFragmentProcessor.cpp
04:34:52     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:52     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:52     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:52     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:52     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/base'
04:34:53     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/clients/api'
04:34:53     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.8.4/VC/bin/Hostx64/x86/cl.exe -FoUnified_cpp_dom_clients_api0.obj -c  -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DWIN32_LEAN_AND_MEAN -D_WIN32 -DWIN32 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DOS_WIN=1 -D_UNICODE -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DUNICODE -D_WINDOWS -D_SECURE_ATL -DCOMPILER_MSVC -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/dom/clients/api -Iz:/build/build/src/obj-firefox/dom/clients/api -Iz:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -Iz:/build/build/src/ipc/chromium/src -Iz:/build/build/src/ipc/glue -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -wd4065 -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -O1 -Oi -Oy- -WX  -deps.deps/Unified_cpp_dom_clients_api0.obj.pp    z:/build/build/src/obj-firefox/dom/clients/api/Unified_cpp_dom_clients_api0.cpp
04:34:53     INFO -  Unified_cpp_dom_clients_api0.cpp
04:34:53     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/clients/api'
04:34:53     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:53     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:53     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:53     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/skia'
04:34:56     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/skia'

Backout: https://hg.mozilla.org/integration/autoland/rev/7e99dc45f560afd931870bd24f3ec60578a3c82b

Comment 6

[Arnaud Bienner]

According to MSDN docs [1], max float value is 3.402823466e+38F, so slightly lower than the constant 3.4028235e38 we are using. I guess that's the cause of the build failure.

I don't think using a constant value directly is very neat, I would prefer that we have a float.MAX_VALUE we could use in the WebIDL.

Andrea: do you know if such thing exists? (even that means modifying the WebAudio spec).
I couldn't find other examples in WebIDL where we are settings a float default value to maximum/minimum possible value.

[1]: https://msdn.microsoft.com/en-us/library/6bs3y5ya.aspx

Comment 7

[Arnaud Bienner]

Note that this build failure might not be an issue anymore once bug 1512504 is implemented (i.e. MSCV build support is dropped).
Still, I'm interested to know if we could better define FLOAT_MAX with something else than a literal value in webidl files.

Comment 8

[Karl Tomlinson (:karlt)]

The "constant too big" error from MSVC seems to be a bug in the compiler AFAICT.

In n4296, section 2.13.4 Floating literals says
"If the scaled value is in the range of representable values for its type, the result is the scaled value if representable, else the larger or smaller representable value nearest the scaled value, chosen in an implementation-defined manner."
"If the scaled value is not in the range of representable values for its type, the program is ill-formed."

If the platform does not support +/- infinity, then the program can be ill-formed, but we are targeting only platforms that do support +/- infinity.
I assume the "implementation-defined" behavior we are expecting here is defined by IEEE 754 round-to-nearest mode.

% print $((2.0**128))
3.4028236692093846e+38
% print $((2.0**127*(2.0-2.0**-23)))
3.4028234663852886e+38

3.4028235e38 is close enough to the maximum finite value that I assume IEEE 754 round-to-nearest mode requires that it is rounded to the maximum finite value.  That assumption is supported by
https://stackoverflow.com/questions/17751031/downcasting-double-to-float-is-overflow-behaviour-guaranteed

Some further non-normative discussion at
https://stackoverflow.com/questions/40694384/integer-to-float-conversions-with-ieee-fp

Comment 9

[Karl Tomlinson (:karlt)]

I considered using a double literal and converting to float to work around this, but that would not be consistent with the "Mathematical Value that would be obtained if S were parsed as an ECMAScript NumericLiteral" language in https://heycam.github.io/webidl/#idl-constants .  It could end up rounding a value twice, which could give a different result.

https://hg.mozilla.org/try/rev/d3c4cc57d8e42af51fdfcde4ad2e8b44b861bcfd

Looking at the implementation and use of numericValue() in Gecko's Codegen.py, I don't see any parsing of webidl constants.  (+/-Infinity and NaN are special literal values.)
https://searchfox.org/mozilla-central/rev/fd62b95c187a40b328d9e7fd9d848833a6942b57/dom/bindings/Codegen.py#4211

The best workaround here I expect is to choose a literal with slightly  different Mathematical Value that would be accepted by MSVC, but would still be expected to round FLT_MAX.  3.402823466e+38F would satisfy the latter at least.
A comment to explain the different value would be helpful and could mention bug 1512504.

Comment 10

[Arnaud Bienner]

I'm a bit lazy and was inclined to wait on bug 1512504 being fixed first to land that patch again, but I'm not sure when that will be.
Karl: if this is blocking you somehow, let me know and I will update my patch with the fix you proposed.

Comment 11

[Arnaud Bienner]

Since I'm not sure how long it will take before we drop support for MSVC, I've updated the patch with Karl' suggestion (+ a FIXME comment).
Pushed to try.

Comment 12

[Arnaud Bienner]

Try is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a96febe89d88de9f6d673dc9dd347f9702d4ccf

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc6fa6ebad86
AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation. r=karlt,baku,froydnj

Comment 14

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/fc6fa6ebad86

Comment 15

[Arnaud Bienner]

Creating a new bug 1519562 since the patch landed was just about getting the descriptors, and the last step remains to be implemented (save them).