Batch configure support for streaming plugin #3239
Conversation
|
Thanks for your contribution, @petarminchev! Please make sure you sign our CLA, as it's a required step before we can merge this. |
There was a problem hiding this comment.
It seems ok, but I haven't tested it yet: I only added a couple of editorial comments. What's definitely missing, though, is documentation: the streams array for "configure" is not documented in the doxygen part of the code, which we did in #2986.
I just noticed though that the "configure" request in the Streaming plugin seems to only do something on RTP (and RTSP) mountpoints, though, which means that the file-based mountpoints were never impacted. This is a note to self, since it means I'll probably have to add something for them too (at the very least to tweak the send property, for instance).
src/plugins/janus_streaming.c
Outdated
| /* Check which properties we need to tweak */ | ||
| const char *mid = json_string_value(json_object_get(sconf, "mid")); | ||
| json_t *send = json_object_get(sconf, "send"); | ||
|
|
There was a problem hiding this comment.
Thanks, removed it in the newest commit.
src/plugins/janus_streaming.c
Outdated
| json_decref(event); | ||
| if(send) | ||
| s->send = json_is_true(send); | ||
| /* FIXME What if we're simulcasting or doing SVC on two different video streams? */ |
There was a problem hiding this comment.
This FIXME message is outdated, now, since the streams approach allows for a per-stream configuration of simulcast/SVC properties. In the VideoRoom we have this comment instead:
/* Check if a simulcasting-related request is involved */
but maybe it should be changes to address the fact it could be SVC too.
There was a problem hiding this comment.
Removed the FIXME comment.
So if I understood correctly nothing needs to be fixed to address the obsolete FIXME comment.
2. Removed empty line and obsolete FIXME comment about simulcast configure
|
Added streams array to the doxygen part of the code. |
|
@petarminchev thanks! I'll make a couple of tests later today, and I've launched the CI checks on the branch too. |
|
The COO of DigitalSamba has already signed the CLA, I suppose that should be enough? Do you see the signed CLA by him? |
|
It must be signed by you: you're the author of the patch. |
|
Signed it |
|
Sorry for the lack of feedback! I plan to make a couple of tests later today and merge in case it's fine 👍 |
|
Made a few tests with a multistream mountpoint with multiple simulcast video streams, and it does seem to work as expected 👍 I vedified that a "legacy" configure request with no mid seems to be enforced on all streams, which means it is backwards compatible too. Merging then, thanks again! |
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. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4zNC4wIiwidXBkYXRlZEluVmVyIjoiMzcuODEuNCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9--> 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>
Supporting a streams array param to configure multiple mids in one batch request.