[1.x][Audiobridge] Couple minor issues in "enable_mjrs" request handling

[RSATom]

First issue is very minor:

janus-gateway/src/plugins/janus_audiobridge.c

Lines 3823 to 3833 in 961b576

	gboolean room_prev_mjrs_active = mjrs_active;
	if(mjrs_active && mjrsdir) {
	/* Update the path where to save the MJR files */
	char *old_mjrs_dir = audiobridge->mjrs_dir;
	char *new_mjrs_dir = g_strdup(json_string_value(mjrsdir));
	audiobridge->mjrs_dir = new_mjrs_dir;
	g_free(old_mjrs_dir);
	}
	if(room_prev_mjrs_active != audiobridge->mjrs) {
	/* Room recording state has changed */
	audiobridge->mjrs = room_prev_mjrs_active;

it looks like room_prev_mjrs_active can be removed safely and replaced directly with mjrs_active.

Second issue is almost in the same place:

janus-gateway/src/plugins/janus_audiobridge.c

Lines 3824 to 3830 in 961b576

	if(mjrs_active && mjrsdir) {
	/* Update the path where to save the MJR files */
	char *old_mjrs_dir = audiobridge->mjrs_dir;
	char *new_mjrs_dir = g_strdup(json_string_value(mjrsdir));
	audiobridge->mjrs_dir = new_mjrs_dir;
	g_free(old_mjrs_dir);
	}

i.e. If incoming enable_mjrs request contains only mjrs_dir but not mjrs - dir will not be updated. And if next enable_mjrs will not contain msrs_dir but only mjrs = true, recording will start to wrong dir.

[RSATom]

And btw I didn't find enable_mjrs request in docs 🤔

[lminiero]

And btw I didn't find enable_mjrs request in docs thinking

You should look harder, then, it is there 😝

https://github.com/meetecho/janus-gateway/blob/master/src/plugins/janus_audiobridge.c#L269-L285

I'll have a look at the code with respect to your other notes. IIRC, at the time that variable was meant to detect whether there had been a change, but we may have changed the logic since the first PR.

[RSATom]

You should look harder, then, it is there 😝

Oops, my fault, I think I was looking to videoroom docs 🤦‍♂️ Sorry.

[lminiero]

I'll have a look at the code with respect to your other notes. IIRC, at the time that variable was meant to detect whether there had been a change, but we may have changed the logic since the first PR.

Looks like this dates back to #2674, which we used as a basis for the enable_mjrs request: as such, it's likely the same problem happens when enabling recordings in general too (and I guess in the VideoRoom as well, which also has enable_recording as part of its API). I think the main problem is that those variables with prev in the name are improperly called, and can lead to confusion: about the dir change, not sure if we should always allow it (getting rid of that extra check) or if we should only allow it either if mjr recording was active before, or if we're activating it now (which would mean extending the extra check).

[lminiero]

Fixed enable_mjrs in both master and 0.x, since it just updates a string. No need for now to do the same for enable_recording, since in that case that actually results in a creation of folders.