Support larger values for SDP attributes #3282

petarminchev · 2023-10-24T16:00:42Z

Currently the limit is 512 bytes for the SDP attribute value, which limits to maximum 75 video + audio mids (150 mids).
Increased limit to 2048 bytes, which should support up to around 267 video + audio mids (534 mids)

2. Removed empty line and obsolete FIXME comment about simulcast configure

# Conflicts: # src/plugins/janus_streaming.c

…s instead of only 75.

lminiero

Adding a note inline.

lminiero · 2023-10-27T16:12:39Z

src/sdp-utils.c

@@ -1119,7 +1119,7 @@ char *janus_sdp_write(janus_sdp *imported) {
 	if(!imported)
 		return NULL;
 	janus_refcount_increase(&imported->ref);
-	char *sdp = g_malloc(1024), mline[8192], buffer[512];
+	char *sdp = g_malloc(1024), mline[8192], buffer[2048];


If the buffer is this large, now, then I guess the sdp allocation we start from should be larger too? We only reallocate it when appending m-lines, which means it should be large enough to handle global attributes before that.

You are right, should then the sdplen variable match the g_malloc(size)?
So I should change to *char sdp = g_malloc(4096) and size_t sdplen = 4096?

@lminiero - Committed an increase to 4096 of initial g_malloc and size_t sdplen

Mh I think it might be overkill, the vast majority of SDPs are never that large: I think we started from a lower value on purpose. I'll give this some thought and get back to you later on.

To be more conservative, maybe let's keep the same diff that was before, so 2048+512=2560? It's still large, but not THAT large.

Agreed, reduced to 2560 sdplen and g_malloc(size)

…rily huge.

lminiero · 2023-11-20T12:28:13Z

Thanks! I think this is good to merge 🆗

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [meetecho/janus-gateway](https://github.com/meetecho/janus-gateway) | minor | `v1.1.4` -> `v1.2.1` | --- ### Release Notes <details> <summary>meetecho/janus-gateway (meetecho/janus-gateway)</summary> ### [`v1.2.1`](https://github.com/meetecho/janus-gateway/blob/HEAD/CHANGELOG.md#v121---2023-12-06) [Compare Source](meetecho/janus-gateway@v1.2.0...v1.2.1) - Added support for abs-capture-time RTP extension \[[PR-3161](meetecho/janus-gateway#3161)] - Fixed truncated label in datachannels (thanks [@veeting](https://github.com/veeting)!) \[[PR-3265](meetecho/janus-gateway#3265)] - Support larger values for SDP attributes (thanks [@petarminchev](https://github.com/petarminchev)!) \[[PR-3282](meetecho/janus-gateway#3282)] - Fixed rare crash in VideoRoom plugin (thanks [@tmatth](https://github.com/tmatth)!) \[[PR-3259](meetecho/janus-gateway#3259)] - Don't create VideoRoom subscriptions to publisher streams with no associated codecs - Added suspend/resume participant API to AudioBridge \[[PR-3301](meetecho/janus-gateway#3301)] - Fixed rare crash in AudioBridge - Fixed rare crash in Streaming plugin when doing ICE restarts \[[Issue-3288](meetecho/janus-gateway#3288)] - Allow SIP and NoSIP plugins to bind media to a specific address (thanks [@pawnnail](https://github.com/pawnnail)!) \[[PR-3263](meetecho/janus-gateway#3263)] - Removed advertised support for SIP UPDATE in SIP plugin - Parse RFC2833 DTMF to custom events in SIP plugin (thanks [@ywmoyue](https://github.com/ywmoyue)!) \[[PR-3280](meetecho/janus-gateway#3280)] - Fixed missing Contact header in some dialogs in SIP plugin (thanks [@ycherniavskyi](https://github.com/ycherniavskyi)!) \[[PR-3286](meetecho/janus-gateway#3286)] - Properly set mid when notifying about ended tracks in janus.js - Fixed broken restamping in janus-pp-rec - Other smaller fixes and improvements (thanks to all who contributed pull requests and reported issues!) ### [`v1.2.0`](https://github.com/meetecho/janus-gateway/blob/HEAD/CHANGELOG.md#v120---2023-08-09) [Compare Source](meetecho/janus-gateway@v1.1.4...v1.2.0) - Added support for VP9/AV1 simulcast, and fixed broken AV1/SVC support \[[PR-3218](meetecho/janus-gateway#3218)] - Fixed RTCP out quality stats \[[PR-3228](meetecho/janus-gateway#3228)] - Default link quality stats to 100 - Added support for ICE consent freshness \[[PR-3234](meetecho/janus-gateway#3234)] - Fixed rare race condition in VideoRoom \[[PR-3219](meetecho/janus-gateway#3219)] \[[PR-3247](meetecho/janus-gateway#3247)] - Use speexdsp's jitter buffer in the AudioBridge \[[PR-3233](meetecho/janus-gateway#3233)] - Fixed crash in Streaming plugin on mountpoints with too many streams \[[Issue-3225](meetecho/janus-gateway#3225)] - Support for batched configure requests in Streaming plugin (thanks [@petarminchev](https://github.com/petarminchev)!) \[[PR-3239](meetecho/janus-gateway#3239)] - Fixed rare deadlock in Streamin plugin \[[PR-3250](meetecho/janus-gateway#3250)] - Fix simulated leave message for longer string ID rooms in TextRoom (thanks [@adnanel](https://github.com/adnanel)!) [PR-3243](meetecho/janus-gateway#3243)] - Notify about count of sessions, handles and PeerConnections on a regular basis, when event handlers are enabled \[[PR-3221](meetecho/janus-gateway#3221)] - Fixed broken Insertable Streams for recvonly streams when answering in janus.js - Added background selector and blur support to Virtual Background demo - Other smaller fixes and improvements (thanks to all who contributed pull requests and reported issues!) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.walbeck.it/walbeck-it/docker-janus-gateway/pulls/122 Co-authored-by: renovate-bot <bot@walbeck.it> Co-committed-by: renovate-bot <bot@walbeck.it>

petarminchev and others added 7 commits June 28, 2023 18:34

Batch configure for streaming plugin

2c339c1

Merge branch 'meetecho:master' into master

58679f9

1. Added documentation of streams array

d5f6fc4

2. Removed empty line and obsolete FIXME comment about simulcast configure

Merge branch 'master' of https://github.com/meetecho/janus-gateway

44ca0fd

# Conflicts: # src/plugins/janus_streaming.c

Increase buffer size of SDP attribute value to support around 267 mid…

15b7868

…s instead of only 75.

Increase buffer size on chained mids.

bffec3d

Fix one more occurrence of small [512] buffer when writing SDP

bb7f03b

petarminchev mentioned this pull request Oct 24, 2023

[1.x] Janus cannot handle more than 150 mids in a subscription #3276

Closed

lminiero reviewed Oct 27, 2023

View reviewed changes

Increased initial SDP allocation size to 4096

de9c79a

lminiero added the multistream Related to Janus 1.x label Nov 16, 2023

Reduced initial SDP allocation size to 2560, since 4096 was unnecessa…

7c3edc4

…rily huge.

lminiero merged commit d2b829b into meetecho:master Nov 20, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support larger values for SDP attributes #3282

Support larger values for SDP attributes #3282

petarminchev commented Oct 24, 2023

lminiero left a comment

lminiero Oct 27, 2023

petarminchev Oct 27, 2023

petarminchev Oct 30, 2023

lminiero Nov 4, 2023

lminiero Nov 20, 2023

petarminchev Nov 20, 2023

lminiero commented Nov 20, 2023

Support larger values for SDP attributes #3282

Support larger values for SDP attributes #3282

Conversation

petarminchev commented Oct 24, 2023

lminiero left a comment

Choose a reason for hiding this comment

lminiero Oct 27, 2023

Choose a reason for hiding this comment

petarminchev Oct 27, 2023

Choose a reason for hiding this comment

petarminchev Oct 30, 2023

Choose a reason for hiding this comment

lminiero Nov 4, 2023

Choose a reason for hiding this comment

lminiero Nov 20, 2023

Choose a reason for hiding this comment

petarminchev Nov 20, 2023

Choose a reason for hiding this comment

lminiero commented Nov 20, 2023