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 (networked-aframe/minijanus.js#2).

package-lock.json changes due to npm ci conflicts

[vincentfretin]

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?

[vincentfretin]

Can you use setEventLoops so that it matches the config in janus?

[vincentfretin]

Put a space before the "?" please.

[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]

networked-aframe/minijanus.js#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 #49 (comment) in commit b6e1bd7

I cherry-picked the two commits in 3.0.x branch 07616e3 and f98ab69

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