Add loop_index configuration for janus

[hjopel]

This PR adds loops for the loop_index configuration to pass to the attach function of the minijanus (https://github.com/networked-aframe/minijanus.js/pull/2).

package-lock.json changes due to npm ci conflicts

[vincentfretin]

I'll take care of backporting that to the 3.0.x branch once this is merged. I'm using 3.0.x in production currently.

[arpu]

I'll take care of backporting that to the 3.0.x branch once this is merged. I'm using 3.0.x in production currently.

@vincentfretin on this topic, i think it would be good to skip 4.x release and release 5.x based on 3.0.x with this changes?

( we use 3.0.x in production)

[vincentfretin]

This is not a major change, we can release @networked-aframe/naf-janus-adapter 3.1.0. FYI I can't release on existing naf-janus-adapter on npm, I don't have the right and I'll never have them.

[hjopel]

Thank you! I also merged #53 as discussed with @arpu I'll update the minijanus dependency in package.json once networked-aframe/minijanus.js#2 and this PR is merged.

Did you prepare a PR for https://github.com/networked-aframe/janus-plugin-sfu to modify the start.sh script in the docker image to configure event_loops and allow_loop_indication with environment variables?

I haven't prepared one as I figured having it in the config https://github.com/networked-aframe/janus-plugin-sfu/blob/master/docker/confs/janus.jcfg#L91 is enough. Should I prepare one? If yes, I assume it should be the same as e.g. NETWORK_INTERFACE? https://github.com/networked-aframe/janus-plugin-sfu/blob/master/docker/start.sh#LL8C1-L18C3

[vincentfretin]

Adding support for environment variables will allow to experiment quickly without rebuilding the image. You can indeed do something similar to NETWORK_INTERFACE, with a check "not empty" with ! -z, this should do it:

if [ ! -z "$EVENT_LOOPS" ]; then
  sed -i \
         -e "s|#event_loops =.*|event_loops = ${EVENT_LOOPS}|" \
         /usr/etc/janus/janus.jcfg
fi
if [ ! -z "$ALLOW_LOOP_INDICATION" ]; then
  sed -i \
         -e "s|#allow_loop_indication =.*|allow_loop_indication = ${ALLOW_LOOP_INDICATION}|" \
         /usr/etc/janus/janus.jcfg
fi

Can you please test and create the PR? Thanks.

[vincentfretin]

And also please a note about those new variables in https://github.com/networked-aframe/janus-plugin-sfu/blob/master/docker/README.md

[hjopel]

makes sense, will look into it

[vincentfretin]

https://github.com/networked-aframe/minijanus.js/pull/2 is merged. https://www.npmjs.com/package/@networked-aframe/minijanus 0.7.0 released. You can change in package.json

"minijanus": "0.6.2"

by

"@networked-aframe/minijanus": "0.7.0"

and change require("minijanus"); by require("@networked-aframe/minijanus"); in index.js

[vincentfretin]

I was waiting you to do the change, not sure if this was clear? Anyway, I merged it, and did the change https://github.com/networked-aframe/naf-janus-adapter/pull/49#issuecomment-1589071096 in commit https://github.com/networked-aframe/naf-janus-adapter/commit/b6e1bd71e94b41e3d689d5a17e0c2bfca7a8f94d

I cherry-picked the two commits in 3.0.x branch https://github.com/networked-aframe/naf-janus-adapter/commit/07616e353d835477c8bbb12e637d4410f2e48504 and https://github.com/networked-aframe/naf-janus-adapter/commit/f98ab69e21119c5fb4115588c7a8df46a47c896e

Congrats for your first contribution in this repo!

[vincentfretin]

@networked-aframe/naf-janus-adapter 3.1.0 and 4.1.0 released with those changes.

[hjopel]

thank you & apologies, it fell under my radar as I've got a bit on my plate right now. appreciate it