New VideoRoom request to combine subscribe and unsubscribe

[lminiero]

If you're familiar with how the new multistream-based VideoRoom API works, once a subscription PeerConnection has been created at the moment there are only two requests available to change it:

1. subscribe to subscribe to new streams (e.g., users joining);
2. unsubscribe to unsubscribe from streams you're currently receiving (e.g., users leaving).

This means that, to update a subscription, you have to use one or the other: if you want to both add new streams AND remove existing ones, you're forced to call those requests in sequence, which can result in an increased number of SDP offer/answer exchanges, and a more awkward streams management.

As the title suggests, this PR tries to solve that by adding a new request for subscribers, called update, whose purpose is exactly to allow you to change an existing subscriptions to both add some new streams and remove existing ones at the same time. The syntax of this new update request is quite simple, since it works like this:

{
	"request" : "update",
	"subscribe" : [
		{
			"feed" : <unique ID of publisher owning the new stream to subscribe to>,
			"mid" : "<unique mid of the publisher stream to subscribe to; optional>"
			// Optionally, simulcast or SVC targets (defaults if missing)
		},
		// Other new streams to subscribe to
	],
	"unsubscribe" : [
		{
			"feed" : <unique ID of publisher owning the new stream to unsubscribe from; optional>,
			"mid" : "<unique mid of the publisher stream to unsubscribe from; optional>"
			"sub_mid" : "<unique mid of the subscriber stream to unsubscribe; optional>"
		},
		// Other streams to unsubscribe from
	]
}

This means that the subscribe array works exactly like the streams array you'd send in a "subscribe" request, while the unsubscribe array works exactly like the streams array you'd send in a "unsubscribe" request instead. Both arrays are optional, which means that sending an update with just a subscribe array is functionally exactly the same as sending a "subscribe" request, and same for "unsubscribe": considering the format of those arrays is exactly the same too, it means those requests do become interchangeable, especially considering that the response is the same no matter what request you send (i.e., an "updated" event with new SDP offer and new list of streams, or an empty "updating" event if the new offer will be sent later).

Of course, the greatest added benefit is the ability to pass both arrays at the same time, which will allow you to update a subscription with much finer grain control. It's important to point out that, when processing an update request, the plugin will always handle the unsubscribe list, and the subscribe one.

This PR is based on #2958 as they're part of the same set of enhancements we've been working on, and so as a consequence this means that if you test this PR, you can test dummy publishers too at the same time. When they're ready, we'll merge them both at the same time.

From a few tests I made, this seems to be working correctly for all requests: considering this considerably changes how the pre-existing subscribe and unsubscribe requests now work internally, you'll want to make sure there are no regressions caused by this patch before we merge. As such, I strongly encourage you all to test this and provide feedback, especially if you use the multistream-based VideoRoom a lot.

[mirkobrankovic]

lminiero wants to merge 1 commit into dummy-publisher from combined-subunsub
Does this mean we need to wait for both PRs to be merged to master?
Thanks

[lminiero]

Does this mean we need to wait for both PRs to be merged to master?

No, this means that this PR is based on that other PR (which is based on master), so testing one you actually test both. But the plan is indeed to merge them together when they're ready, yes.

[mirkobrankovic]

we are testing it now thanks to changes by @stefanzvkvc.
So far no issues, would be nice to have section to combine configure request also for changes on the stream, but I guess that is a completely different parts of the plugin so will make everything more complex in janus plugin.
(configure would mean only changing substream layers, not more complex stuff )
This already simplified our code quite enough.

[lminiero]

Thanks for the feedback @mirkobrankovic! I was planning to merge both PRs today, can you confirm you didn't encounter any issue we need to be aware of using it?

[lminiero]

would be nice to have section to combine configure request also for changes on the stream

Do you mean in configure requests? That indeed something I'm thinking of, specifically the ability to pass an array of changes, vs only being able to change a single mid per request. I'll probably start doing that after these two are merged.

[mirkobrankovic]

@lminiero no issues so far, we have a case (when screensharing starts) when we send update with subscribe and unsubscribe streams and MIDs are correctly recycled

great news.

Yeah I was thinking a long the way of current update:

{
	"request" : "update",
	"subscribe" : [
		{
			"feed" : <unique ID of publisher owning the new stream to subscribe to>,
			"mid" : "<unique mid of the publisher stream to subscribe to; optional>"
		},
		// Other new streams to subscribe to
	],
	"unsubscribe" : [
		{
			"feed" : <unique ID of publisher owning the new stream to unsubscribe from; optional>,
			"mid" : "<unique mid of the publisher stream to unsubscribe from; optional>"
			"sub_mid" : "<unique mid of the subscriber stream to unsubscribe; optional>"
		},
		// Other streams to unsubscribe from
	],
        "configure": [
		{
			"feed" : <unique ID of publisher owning the new stream to unsubscribe from; optional>,
			"mid" : "<unique mid of the publisher stream to unsubscribe from; optional>"
			"substream" : 2,
                        "temporal" : 2
		},
		// Other streams to configure
	],
}

so to re-configure the substream layers, not sure what else could be possible, if it is it does sound interesting to be combined.

Thanks

[lminiero]

Ah no that's not planned at the moment. I was talking of the existing configure request, where right now you have to pass a mid to specify the single m-line you want to update, and I want to allow more of them to be changed at the same time. The syntax you suggest may be an idea for the future, after that happens.

[mirkobrankovic]

aah ok, configure for multiple mids is also a good step forward, thanks for update :)

[lminiero]

Merging then.