-
Notifications
You must be signed in to change notification settings - Fork 18
Allow developers to control the audio codecs #1190
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.
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
Conversation
🦋 Changeset detectedLatest commit: bd5996f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
From https://datatracker.ietf.org/doc/html/rfc7587: It may make sense to let the user pick one of those |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay to me. 👍 I have a left a few questions.
I would give more weight to @giavac's review on this.
| const SDP_OPUS_STEREO = | ||
| 'v=0\r\no=- 135160591336882782 2 IN IP4 127.0.0.1\r\ns=-\r\nt=0 0\r\na=group:BUNDLE audio video\r\na=msid-semantic: WMS 381b9efc-7cf5-45bb-8f39-c06558b288de\r\nm=audio 9 UDP/TLS/RTP/SAVPF 111 103 104 9 0 8 106 105 13 110 112 113 126\r\nc=IN IP4 0.0.0.0\r\na=rtcp:9 IN IP4 0.0.0.0\r\na=ice-ufrag:Y5Zy\r\na=ice-pwd:yQLVrXgG+irP0tgLLr4ZjQb5\r\na=ice-options:trickle\r\na=fingerprint:sha-256 45:ED:86:FB:EB:FE:21:20:62:C4:07:81:AA:B8:BC:87:60:CC:2B:54:CE:D5:F0:16:93:C4:61:23:28:59:DF:8B\r\na=setup:actpass\r\na=mid:audio\r\na=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level\r\na=sendrecv\r\na=rtcp-mux\r\na=rtpmap:111 opus/48000/2\r\na=rtcp-fb:111 transport-cc\r\na=fmtp:111 minptime=10;useinbandfec=1; stereo=1; sprop-stereo=1\r\na=rtpmap:103 ISAC/16000\r\na=rtpmap:104 ISAC/32000\r\na=rtpmap:9 G722/8000\r\na=rtpmap:0 PCMU/8000\r\na=rtpmap:8 PCMA/8000\r\na=rtpmap:106 CN/32000\r\na=rtpmap:105 CN/16000\r\na=rtpmap:13 CN/8000\r\na=rtpmap:110 telephone-event/48000\r\na=rtpmap:112 telephone-event/32000\r\na=rtpmap:113 telephone-event/16000\r\na=rtpmap:126 telephone-event/8000\r\na=ssrc:3652058873 cname:kufSZ8JnlRUuQVc2\r\na=ssrc:3652058873 msid:381b9efc-7cf5-45bb-8f39-c06558b288de 8841cbb1-90ba-4655-8784-60a185846706\r\na=ssrc:3652058873 mslabel:381b9efc-7cf5-45bb-8f39-c06558b288de\r\na=ssrc:3652058873 label:8841cbb1-90ba-4655-8784-60a185846706\r\nm=video 9 UDP/TLS/RTP/SAVPF 96 97 98 99 100 101 102 122 127 121 125 107 108 109 124 120 123 119 114\r\nc=IN IP4 0.0.0.0\r\na=rtcp:9 IN IP4 0.0.0.0\r\na=ice-ufrag:Y5Zy\r\na=ice-pwd:yQLVrXgG+irP0tgLLr4ZjQb5\r\na=ice-options:trickle\r\na=fingerprint:sha-256 45:ED:86:FB:EB:FE:21:20:62:C4:07:81:AA:B8:BC:87:60:CC:2B:54:CE:D5:F0:16:93:C4:61:23:28:59:DF:8B\r\na=setup:actpass\r\na=mid:video\r\na=extmap:2 urn:ietf:params:rtp-hdrext:toffset\r\na=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\r\na=extmap:4 urn:3gpp:video-orientation\r\na=extmap:5 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01\r\na=extmap:6 http://www.webrtc.org/experiments/rtp-hdrext/playout-delay\r\na=extmap:7 http://www.webrtc.org/experiments/rtp-hdrext/video-content-type\r\na=extmap:8 http://www.webrtc.org/experiments/rtp-hdrext/video-timing\r\na=extmap:10 http://tools.ietf.org/html/draft-ietf-avtext-framemarking-07\r\na=sendrecv\r\na=rtcp-mux\r\na=rtcp-rsize\r\na=rtpmap:96 VP8/90000\r\na=rtcp-fb:96 goog-remb\r\na=rtcp-fb:96 transport-cc\r\na=rtcp-fb:96 ccm fir\r\na=rtcp-fb:96 nack\r\na=rtcp-fb:96 nack pli\r\na=rtpmap:97 rtx/90000\r\na=fmtp:97 apt=96\r\na=rtpmap:98 VP9/90000\r\na=rtcp-fb:98 goog-remb\r\na=rtcp-fb:98 transport-cc\r\na=rtcp-fb:98 ccm fir\r\na=rtcp-fb:98 nack\r\na=rtcp-fb:98 nack pli\r\na=fmtp:98 profile-id=0\r\na=rtpmap:99 rtx/90000\r\na=fmtp:99 apt=98\r\na=rtpmap:100 H264/90000\r\na=rtcp-fb:100 goog-remb\r\na=rtcp-fb:100 transport-cc\r\na=rtcp-fb:100 ccm fir\r\na=rtcp-fb:100 nack\r\na=rtcp-fb:100 nack pli\r\na=fmtp:100 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f\r\na=rtpmap:101 rtx/90000\r\na=fmtp:101 apt=100\r\na=rtpmap:102 H264/90000\r\na=rtcp-fb:102 goog-remb\r\na=rtcp-fb:102 transport-cc\r\na=rtcp-fb:102 ccm fir\r\na=rtcp-fb:102 nack\r\na=rtcp-fb:102 nack pli\r\na=fmtp:102 level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42001f\r\na=rtpmap:122 rtx/90000\r\na=fmtp:122 apt=102\r\na=rtpmap:127 H264/90000\r\na=rtcp-fb:127 goog-remb\r\na=rtcp-fb:127 transport-cc\r\na=rtcp-fb:127 ccm fir\r\na=rtcp-fb:127 nack\r\na=rtcp-fb:127 nack pli\r\na=fmtp:127 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f\r\na=rtpmap:121 rtx/90000\r\na=fmtp:121 apt=127\r\na=rtpmap:125 H264/90000\r\na=rtcp-fb:125 goog-remb\r\na=rtcp-fb:125 transport-cc\r\na=rtcp-fb:125 ccm fir\r\na=rtcp-fb:125 nack\r\na=rtcp-fb:125 nack pli\r\na=fmtp:125 level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42e01f\r\na=rtpmap:107 rtx/90000\r\na=fmtp:107 apt=125\r\na=rtpmap:108 H264/90000\r\na=rtcp-fb:108 goog-remb\r\na=rtcp-fb:108 transport-cc\r\na=rtcp-fb:108 ccm fir\r\na=rtcp-fb:108 nack\r\na=rtcp-fb:108 nack pli\r\na=fmtp:108 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=4d0032\r\na=rtpmap:109 rtx/90000\r\na=fmtp:109 apt=108\r\na=rtpmap:124 H264/90000\r\na=rtcp-fb:124 goog-remb\r\na=rtcp-fb:124 transport-cc\r\na=rtcp-fb:124 ccm fir\r\na=rtcp-fb:124 nack\r\na=rtcp-fb:124 nack pli\r\na=fmtp:124 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=640032\r\na=rtpmap:120 rtx/90000\r\na=fmtp:120 apt=124\r\na=rtpmap:123 red/90000\r\na=rtpmap:119 rtx/90000\r\na=fmtp:119 apt=123\r\na=rtpmap:114 ulpfec/90000\r\na=ssrc-group:FID 1714381393 967654061\r\na=ssrc:1714381393 cname:kufSZ8JnlRUuQVc2\r\na=ssrc:1714381393 msid:381b9efc-7cf5-45bb-8f39-c06558b288de 99d2faa8-950d-40f7-ad80-16789c9b4faa\r\na=ssrc:1714381393 mslabel:381b9efc-7cf5-45bb-8f39-c06558b288de\r\na=ssrc:1714381393 label:99d2faa8-950d-40f7-ad80-16789c9b4faa\r\na=ssrc:967654061 cname:kufSZ8JnlRUuQVc2\r\na=ssrc:967654061 msid:381b9efc-7cf5-45bb-8f39-c06558b288de 99d2faa8-950d-40f7-ad80-16789c9b4faa\r\na=ssrc:967654061 mslabel:381b9efc-7cf5-45bb-8f39-c06558b288de\r\na=ssrc:967654061 label:99d2faa8-950d-40f7-ad80-16789c9b4faa\r\n' | ||
| const SDP_OPUS_NO_STEREO = | ||
| 'v=0\r\no=- 135160591336882782 2 IN IP4 127.0.0.1\r\ns=-\r\nt=0 0\r\na=msid-semantic: WMS 381b9efc-7cf5-45bb-8f39-c06558b288de\r\na=group:BUNDLE audio video\r\nm=audio 9 UDP/TLS/RTP/SAVPF 111 103 104 9 0 8 106 105 13 110 112 113 126\r\nc=IN IP4 0.0.0.0\r\na=rtpmap:111 opus/48000/2\r\na=rtpmap:103 ISAC/16000\r\na=rtpmap:104 ISAC/32000\r\na=rtpmap:9 G722/8000\r\na=rtpmap:0 PCMU/8000\r\na=rtpmap:8 PCMA/8000\r\na=rtpmap:106 CN/32000\r\na=rtpmap:105 CN/16000\r\na=rtpmap:13 CN/8000\r\na=rtpmap:110 telephone-event/48000\r\na=rtpmap:112 telephone-event/32000\r\na=rtpmap:113 telephone-event/16000\r\na=rtpmap:126 telephone-event/8000\r\na=fmtp:111 minptime=10;useinbandfec=1;stereo=1;sprop-stereo=1\r\na=rtcp:9 IN IP4 0.0.0.0\r\na=rtcp-fb:111 transport-cc\r\na=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level\r\na=setup:actpass\r\na=mid:audio\r\na=sendrecv\r\na=ice-ufrag:Y5Zy\r\na=ice-pwd:yQLVrXgG+irP0tgLLr4ZjQb5\r\na=fingerprint:sha-256 45:ED:86:FB:EB:FE:21:20:62:C4:07:81:AA:B8:BC:87:60:CC:2B:54:CE:D5:F0:16:93:C4:61:23:28:59:DF:8B\r\na=ice-options:trickle\r\na=ssrc:3652058873 cname:kufSZ8JnlRUuQVc2\r\na=ssrc:3652058873 msid:381b9efc-7cf5-45bb-8f39-c06558b288de 8841cbb1-90ba-4655-8784-60a185846706\r\na=ssrc:3652058873 mslabel:381b9efc-7cf5-45bb-8f39-c06558b288de\r\na=ssrc:3652058873 label:8841cbb1-90ba-4655-8784-60a185846706\r\na=rtcp-mux\r\nm=video 9 UDP/TLS/RTP/SAVPF 96 97 98 99 100 101 102 122 127 121 125 107 108 109 124 120 123 119 114\r\nc=IN IP4 0.0.0.0\r\na=rtpmap:96 VP8/90000\r\na=rtpmap:97 rtx/90000\r\na=rtpmap:98 VP9/90000\r\na=rtpmap:99 rtx/90000\r\na=rtpmap:100 H264/90000\r\na=rtpmap:101 rtx/90000\r\na=rtpmap:102 H264/90000\r\na=rtpmap:122 rtx/90000\r\na=rtpmap:127 H264/90000\r\na=rtpmap:121 rtx/90000\r\na=rtpmap:125 H264/90000\r\na=rtpmap:107 rtx/90000\r\na=rtpmap:108 H264/90000\r\na=rtpmap:109 rtx/90000\r\na=rtpmap:124 H264/90000\r\na=rtpmap:120 rtx/90000\r\na=rtpmap:123 red/90000\r\na=rtpmap:119 rtx/90000\r\na=rtpmap:114 ulpfec/90000\r\na=fmtp:97 apt=96\r\na=fmtp:98 profile-id=0\r\na=fmtp:99 apt=98\r\na=fmtp:100 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f\r\na=fmtp:101 apt=100\r\na=fmtp:102 level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42001f\r\na=fmtp:122 apt=102\r\na=fmtp:127 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f\r\na=fmtp:121 apt=127\r\na=fmtp:125 level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42e01f\r\na=fmtp:107 apt=125\r\na=fmtp:108 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=4d0032\r\na=fmtp:109 apt=108\r\na=fmtp:124 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=640032\r\na=fmtp:120 apt=124\r\na=fmtp:119 apt=123\r\na=rtcp:9 IN IP4 0.0.0.0\r\na=rtcp-fb:96 goog-remb\r\na=rtcp-fb:96 transport-cc\r\na=rtcp-fb:96 ccm fir\r\na=rtcp-fb:96 nack\r\na=rtcp-fb:96 nack pli\r\na=rtcp-fb:98 goog-remb\r\na=rtcp-fb:98 transport-cc\r\na=rtcp-fb:98 ccm fir\r\na=rtcp-fb:98 nack\r\na=rtcp-fb:98 nack pli\r\na=rtcp-fb:100 goog-remb\r\na=rtcp-fb:100 transport-cc\r\na=rtcp-fb:100 ccm fir\r\na=rtcp-fb:100 nack\r\na=rtcp-fb:100 nack pli\r\na=rtcp-fb:102 goog-remb\r\na=rtcp-fb:102 transport-cc\r\na=rtcp-fb:102 ccm fir\r\na=rtcp-fb:102 nack\r\na=rtcp-fb:102 nack pli\r\na=rtcp-fb:127 goog-remb\r\na=rtcp-fb:127 transport-cc\r\na=rtcp-fb:127 ccm fir\r\na=rtcp-fb:127 nack\r\na=rtcp-fb:127 nack pli\r\na=rtcp-fb:125 goog-remb\r\na=rtcp-fb:125 transport-cc\r\na=rtcp-fb:125 ccm fir\r\na=rtcp-fb:125 nack\r\na=rtcp-fb:125 nack pli\r\na=rtcp-fb:108 goog-remb\r\na=rtcp-fb:108 transport-cc\r\na=rtcp-fb:108 ccm fir\r\na=rtcp-fb:108 nack\r\na=rtcp-fb:108 nack pli\r\na=rtcp-fb:124 goog-remb\r\na=rtcp-fb:124 transport-cc\r\na=rtcp-fb:124 ccm fir\r\na=rtcp-fb:124 nack\r\na=rtcp-fb:124 nack pli\r\na=extmap:2 urn:ietf:params:rtp-hdrext:toffset\r\na=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\r\na=extmap:4 urn:3gpp:video-orientation\r\na=extmap:5 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01\r\na=extmap:6 http://www.webrtc.org/experiments/rtp-hdrext/playout-delay\r\na=extmap:7 http://www.webrtc.org/experiments/rtp-hdrext/video-content-type\r\na=extmap:8 http://www.webrtc.org/experiments/rtp-hdrext/video-timing\r\na=extmap:10 http://tools.ietf.org/html/draft-ietf-avtext-framemarking-07\r\na=setup:actpass\r\na=mid:video\r\na=sendrecv\r\na=ice-ufrag:Y5Zy\r\na=ice-pwd:yQLVrXgG+irP0tgLLr4ZjQb5\r\na=fingerprint:sha-256 45:ED:86:FB:EB:FE:21:20:62:C4:07:81:AA:B8:BC:87:60:CC:2B:54:CE:D5:F0:16:93:C4:61:23:28:59:DF:8B\r\na=ice-options:trickle\r\na=ssrc:1714381393 cname:kufSZ8JnlRUuQVc2\r\na=ssrc:1714381393 msid:381b9efc-7cf5-45bb-8f39-c06558b288de 99d2faa8-950d-40f7-ad80-16789c9b4faa\r\na=ssrc:1714381393 mslabel:381b9efc-7cf5-45bb-8f39-c06558b288de\r\na=ssrc:1714381393 label:99d2faa8-950d-40f7-ad80-16789c9b4faa\r\na=ssrc:967654061 cname:kufSZ8JnlRUuQVc2\r\na=ssrc:967654061 msid:381b9efc-7cf5-45bb-8f39-c06558b288de 99d2faa8-950d-40f7-ad80-16789c9b4faa\r\na=ssrc:967654061 mslabel:381b9efc-7cf5-45bb-8f39-c06558b288de\r\na=ssrc:967654061 label:99d2faa8-950d-40f7-ad80-16789c9b4faa\r\na=ssrc-group:FID 1714381393 967654061\r\na=rtcp-mux\r\na=rtcp-rsize\r\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately, I had to update the SDP values.
One way to check if they still represent the test is to search for fmtp:111 in the opus tests and .local in the candidate's tests.
| @@ -1,556 +1,338 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please run the prettier here? It seems to be a huge change, and I don't understand why. Probably because of some copy-paste.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not getting any prettier change on this file. Even after calling it manually on it.
Isn't that from the default styling of input type number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check if your prettier is configured to run on HTML files?
If I run prettier on this PR, almost the whole HTML file gets updated. Even you check the git diff in your branch; https://github.com/signalwire/signalwire-js/pull/1190/files#diff-754e5cd31ab8b2aa50d4c8bc70ffaf88866ca80db4a77a07a9dcbec89aaf27b0
Isn't that from the default styling of input type number?
Not sure what you mean here. I was highlighting the overflow issue only with this specific field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, what I'm saying is that that field is a number input. And maybe that is the reason why it's overflowing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please run the prettier? It would be easier to review.
packages/js/src/fabric/WSClient.ts
Outdated
|
|
||
| private buildOutboundCall(params: DialParams & { attach?: boolean }) { | ||
| const [pathname, query] = params.to.split('?') | ||
| private _validateDialParams(params: DialParams) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this validator outside of the class? Since it seems to be a utility which does not need a class instance.
Maybe we move it here: packages/js/src/fabric/utils/validateCallDial.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a logic made exclusive for the class should be defined in another module?
this isn't a generic "utility" this is part of the dial() function just refactored to a function to add more semantics to the code.
Do you prefer this code to be inside the dial function?
| private _validateOptions() { | ||
| if ( | ||
| this.options.useStereo === true && | ||
| typeof this.options.audio === 'object' && | ||
| (this.options.audio.channelCount ?? 2) != 2 | ||
| ) { | ||
| throw new Error('Mismatch params: useStereo, audio.channelCount') | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a utility function not associated with the class instance. Should we move this to a utility file?
Also, should we throw an error from the constructor or when the actual API where this is used is called?
If you are expecting this to be passed by the developer, you will need to expose the useStereo on the DialParams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a utility function not associated with the class instance. Should we move this to a utility file?
Why did you have this impression? This utility is to validate the options passed in this class constructor according to the assumptions in this class implementation?
Also, should we throw an error from the constructor or when the actual API where this is used is called?
What actual API? The constructor is the one that receives theoptionsparams.
If you are expecting this to be passed by the developer, you will need to expose the useStereo on the DialParams.
I'm not making assumptions about who sets theuseSteroat this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you have this impression?
The definition of the "utility function".
What actual API?
The actual API that creates this instance. For CF SDK, that would be the dial method.
All the above were questions: if you think that you don't need this, then we can leave it here as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, no need to expose the param to the developer that is not the goal of this PR.
I'm still confused about external "utility" idea...
This is the same case as above. This isn't a generic "utility" this is a "guard" for the assumptions/implementation defined in this class alone.
| return sdp.replace(/\r\n/g, '\n').replace(/\r/g, '\n') | ||
| } | ||
| import { BaseConnectionOptions } from '../BaseConnection' | ||
| import sdpTransform from 'sdp-transform' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you remember what was the API that was missing in the previous library "sdp" and it is available in the new library "sdp-transform"?
Asking because the old lib seems to be widely used in comparison to the new one: https://npmtrends.com/sdp-vs-sdp-transform
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't about a specific API, but about all APIs in general. sdp-transform is both more semantic and lighter at the same time.
I considered the adoption too, but if you check the starts, size, and the recent activity, you will see that sdp-trasform is a better option in general.
:-( Only works with Firefox |
|
@jpsantosbh, could you please check the CI? The Streaming test project is failing constantly in Production. I highlighted this before as well. This might be due to SDP changes, I am not sure. |
@iAmmar7 I noticed that too. And the streaming is unrelated to the SDP. The negotiations are successful |
So, do you know why the streaming test is failing? Maybe we need to ask RTC? It does not seem to be failing in the main branch though. |
|
Yeah, it fails locally for me to with a different error... but it's always an error with the streaming API not a negotiation issue. |
Maybe it is not the negotiation but something else in this PR causing the issue. The last CI run on the main branch passed as well. You can see here: https://github.com/signalwire/signalwire-js/actions/runs/15299726881 There are 4 test files used in the Streaming project:
I would suggest trying to see how many of these are failing, and from there, we can debug failed tests one by one. |
|
I tested locally using the same The 3 🔴 are safe to be ignored. |
@jpsantosbh, thank you for all the changes. I don’t see any specific issues at the moment; however, the PR introduces significant changes to the WebRTC SDP-related utilities. That’s why I pointed it out; it's possible that the SDP changes are contributing to the Production test failures. I'm not sure if we can safely ignore this without notifying the team about the consistent failures in the Production Streaming tests. Ignoring them now could lead to more complex debugging later on. Please let me know if you need any support. I’d be happy to look into this together tomorrow. |
|
Since the test passed locally, let's test if it will continue to fail after merging it to main. |
Sure, up to you, you can resolve the conflicts and merge the PR. |
- Resolved conflict in packages/core/src/utils/index.ts: Added new helper functions from main - Resolved conflict in packages/js/src/fabric/WSClient.ts: Updated buildOutboundCall to use ReattachParams - Resolved conflict in packages/webrtc/package.json: Updated @signalwire/core version but kept sdp-transform dependency - Updated package-lock.json
The work in #1233 will take time. Something has changed in this PR, and it would require time to debug.
I don't merge any PR unless the PROD CI is green. Since you mentioned you want to test and would revert it back, that's why I said it's up to you. |
This reverts commit ba71908.



Description
Added dial params:
The new parameter allows developers to select the audio codec to use with the desired configuration
Replaced the
sdphelper lib withsdpTransformand refactored some of the sdpHelpers code to benefit fromsdpTransformThis allows a more semantic SDP manipulation now that we're exposing to the developer parameters to make changes on the SDP offers.
Type of change
Code snippets