Closed Bug 1501709 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Web Audio, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: arnaud.bienner, Assigned: arnaud.bienner)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

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."
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.
Depends on: 1502004
Depends on: 1503236
Depends on: 1504982
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0e1dbfd10378
AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation. r=karlt,baku,froydnj
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
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
Flags: needinfo?(amarchesini)
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.
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
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.
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.

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.

Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc6fa6ebad86
AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation. r=karlt,baku,froydnj

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Summary: AudioWorkletGlobalScope::RegisterProcessor: save descriptors in a map → AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation
Blocks: 1519562

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

Flags: needinfo?(amarchesini)
Blocks: 1651703
You need to log in before you can comment on or make changes to this bug.