-
Notifications
You must be signed in to change notification settings - Fork 75
Some feedback #50
Comments
Thanks for your feedback! 👍 Your suggestions are very nice and helpful, I think we can add some of them to our team's roadmap. Bundle sizeThere are a lot of babel-polyfill codes in the current SDK bundle. However, these codes are redundant in most scenarios. We build SDK with the polyfill codes because some customers want to import(not use!) our SDK in some very old browsers. (maybe they just want to import our SDK in Adding an PerformanceWe are planning to merge these analytics requests or use WebSocket to report analytics data. CORP & COEPI think it is necessary for Agora service to support CORP & COEP, Audio ContextThis sounds good, I will add this API in the future version. But please be attention that If developers pass their own |
I have tested our demo page in cross-origin isolated mode on Firefox 79, it works fine and I can access the Then I read the spec and find this line:
My understanding is that our service now supports the CORS protocol, so there is no need to add the |
Thanks for the detailed response! Also, your English is really good.
Yeah, manually managing AudioContext is not fun (especially in Safari). Chrome has a little-known API that tells you if creating an AudioContext will work before you actually create it: if (
navigator?.userActivation?.isActive ||
navigator?.userActivation?.hasBeenActive
) {
this.getAudioContext();
} I don't see documentation for
You're right – I misread the document, I thought it required both CORS & CORP rather than either CORS or CORP. I was also missing the Edit: It works in Firefox correctly, but once these headers are added, it no longer works in Chrome – at least, on localhost. This is with the following headers on the page: I'm working around this issue by proxying these domains and editing the code to request relative to const PROXY_DOMAINS = [
"webrtc2-ap-web-1.agora.io",
"webrtc2-ap-web-2.agoraio.cn",
"webrtc2-ap-web-3.agora.io",
"webrtc2-ap-web-4.agoraio.cn",
"ap-proxy-1.agora.io",
"ap-proxy-2.agora.io",
"cds-ap-web-1.agora.io",
"cds-ap-web-2.agoraio.cn",
"cds-ap-web-3.agora.io",
"cds-ap-web-4.agoraio.cn",
"sua-ap-web-1.agora.io",
"sua-ap-web-2.agoraio.cn",
"sua-ap-web-3.agora.io",
"sua-ap-web-4.agoraio.cn",
"uap-ap-web-1.agora.io",
"uap-ap-web-2.agoraio.cn",
"uap-ap-web-3.agora.io",
"uap-ap-web-4.agoraio.cn",
]; BTW, have you seen RNNoise? Its better than native noise cancellation and fast enough to run in real-time on browsers. I integrated it into the game to cover up laptop fan noise in voice chat. I'd be a little wary of using it in browsers that don't support Audio Worklet or any older browsers, but it might be a cool feature to add sometime later since it would (theoretically) improve audio call quality & make the volume level stuff better at distinguishing between speech and ambient noise. On an unrelated note, do you know how to get pushing a livestream to CDN to work? I'm trying to make it push a MediaStreamTrack from a Canvas element to an RTMP url and I get |
It seems that Chrome and Firefox have different CORE strategies. But I think it is still necessary for Agora Service to add this header(there are no side effects in this header I thought). 😃 RNNoise and other software 3A algorithms are in our plan. Chrome's AEC module will not process the audio from WebAudio(https://bugs.chromium.org/p/chromium/issues/detail?id=687574). So we need a way to implement audio 3A processing through JavaScript or WASM. Have you enabled the RTMP Converter service for your APPID in your Agora Console? |
I'm using Agora inside a browser-based game for in-game voice chat. Overall, I really like Agora and Agora Web SDK NG is an easier API than Agora Web SDK – good job!
I have a few points of feedback that I hope is helpful to you.
Bundle size
The minified source is 547 KB, which is large for any library.
I think there are a number of opportunities to reduce bundle size. From looking at the code, you could release 2-3 bundles instead:
If you provided a CommonJS or ESM build and then used
export
&import
in your source code to import files, this would make it more eligible for tree shaking. Additionally, for any libraries you use, you'd require them and add them as a dependency in your package.json, rather than inline them into the source.Different companies use Agora differently – some might use the DOM-related functionality (showing a video stream, adding an
<audio />
tag, etc) and some might not. Tree shaking would allow integrators to only import the parts of Agora's SDK that are relevant to their application. This should significantly reduce bundle size for most integrators.Performance
Agora makes many requests, seemingly for analytics rather than VOIP. There is a meaningful performance cost to sending lots of requests, and browsers have connection limits for how many requests they send at a time per tab.
If the only thing happening on the page is VOIP, then its probably fine. But, if there's lots of other stuff too (e.g. rendering a 3D game), it can cause performance problems.
It would be great if integrators could disable analytics and voice quality-related data collection.
Additionally, allocating new objects or pushing onto arrays inside of
setInterval
(or any timer) is an easy way to trigger Garbage Collection and briefly freeze the webpage. If you reuse objects (where possible) inside of timers, it will improve performance.CORP & COEP
Currently, this SDK does not work when using
Cross-Origin-Resource-Policy
&Cross-Origin-Embedder-Policy
together. Requests to Agora's servers are blocked by the browser.These headers enable
SharedArrayBuffer
in Firefox 79 and soon will be required by Chrome to use SharedArrayBuffer too.These standards are pretty new, so it makes sense why this wouldn't be supported yet. I'm honestly not sure of a solution to this, unless it involves proxying.
Audio Context
Web browsers cannot create many
AudioContext
objects per page without stuff breaking. It would be great if the SDK allowed a developer to pass in their own AudioContext object, instead of creating one during initialization.The text was updated successfully, but these errors were encountered: