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

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P2
normal
RESOLVED FIXED
8 months ago
28 days ago

People

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

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

()

Attachments

(1 attachment)

Assignee

Description

8 months ago
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."
Assignee

Comment 1

8 months ago
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.
Assignee

Updated

8 months ago
Depends on: 1502004
Assignee

Updated

8 months ago
Depends on: 1503236
Assignee

Updated

8 months ago
Depends on: 1504982

Comment 4

7 months ago
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

Comment 5

7 months ago
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
Assignee

Comment 6

7 months ago
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)
Assignee

Comment 7

6 months ago
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.
Assignee

Comment 10

6 months ago
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.
Assignee

Comment 11

5 months ago

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 13

5 months ago

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

Comment 14

5 months ago
bugherder
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Assignee

Updated

5 months ago
Summary: AudioWorkletGlobalScope::RegisterProcessor: save descriptors in a map → AudioWorkletGlobalScope::RegisterProcessor: check descriptors and convert them to an internal representation
Assignee

Updated

5 months ago
Blocks: 1519562
Assignee

Comment 15

5 months ago

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)
You need to log in before you can comment on or make changes to this bug.