[1.x] A code mistaken found in janus_videoroom_subscriber_stream_remove() of videoroom.c

[jamken]

What version of Janus is this happening on?
v1.0.1

Have you tested a more recent version of Janus too?
Yes,

Was this working before?
It's a minor mistaken which is not visable

Is there a gdb or libasan trace of the issue?
I's found in my code review for janus-cloud coding with multisream version

Additional context
janus_videoroom_subscriber_stream_remove() :

static void janus_videoroom_subscriber_stream_remove(janus_videoroom_subscriber_stream *s,
		janus_videoroom_publisher_stream *ps, gboolean lock_ps) {
	janus_videoroom_subscriber *subscriber = s->subscriber;
	if(subscriber && subscriber->pvt_id > 0 && subscriber->room != NULL) {
		janus_mutex_lock(&subscriber->room->mutex);
		janus_videoroom_publisher *owner = g_hash_table_lookup(subscriber->room->private_ids, GUINT_TO_POINTER(subscriber->pvt_id));
		if(owner != NULL) {
			janus_mutex_lock(&owner->subscribers_mutex);
			/* Note: we should refcount these subscription-publisher mappings as well */
			owner->subscriptions = g_slist_remove(owner->subscriptions, s);  //jamken: owner->subscriptions  should be linke with janus_videoroom_subscriber not janus_videoroom_subscriber_stream
			janus_mutex_unlock(&owner->subscribers_mutex);
		}
		janus_mutex_unlock(&subscriber->room->mutex);
		//~ if(subscriber->room)
			//~ g_clear_pointer(&subscriber->room, janus_videoroom_room_dereference);
		//~ /* If the subscriber itself has no more active active subscriptions, should we close it? */
		//~ if(subscriber->streams == NULL && subscriber->session && subscriber->close_pc)
			//~ gateway->close_pc(subscriber->session->handle);
	}

from above code, owner->subscriptions should be linke with janus_videoroom_subscriber but it's remove with a janus_videoroom_subscriber_stream by mistaken
Also, when remove a stream from a subscriber, it make no sense to remove the subsrciber from it's owner

[lminiero]

That's a good catch, but I don't think the fix is as simple as that. If we're just removing a stream, but are still subscribed to other streams, then we should not remove the subscription from that list. I'll need some time to figure out what the right way to handle this.

[lminiero]

The commit above fixes the problem: this was only an issue when forcing require_pvtid: true in a room, but now should be fine in that case as well. Thanks again for spotting this problem!