tiny-pilot / tinypilot

Use your Raspberry Pi as a browser-based KVM.
https://tinypilotkvm.com
MIT License
3.05k stars 255 forks source link

Populate uStreamer Janus plugin config after reading capture device #1568

Closed jdeanwallace closed 1 year ago

jdeanwallace commented 1 year ago

Resolves https://github.com/tiny-pilot/tinypilot/issues/1567

WebRTC audio streaming stopped working since https://github.com/tiny-pilot/tinypilot/pull/1470. The problem was that the uStreamer Janus plugin config file was being written before populating the ustreamer_capture_device variable, which resulted in no Janus audio config: https://github.com/tiny-pilot/tinypilot/blob/b379ac26ea87a99e6126b3d616648b905013cb2f/ansible-role-ustreamer/templates/janus.plugin.ustreamer.jcfg.j2#L5-L10

The ustreamer_capture_device variable is populated when reading the /home/ustreamer/config.yml file: https://github.com/tiny-pilot/tinypilot/blob/b379ac26ea87a99e6126b3d616648b905013cb2f/ansible-role-ustreamer/tasks/check_saved_settings.yml#L11-L14

Back in https://github.com/tiny-pilot/tinypilot/pull/1470, I specifically mentioned that I was shifting where the TC358743 chip was being provisioned, but I incorrectly assumed that only the uStreamer launcher cared about the uStreamer config file:

I've moved the TC358743 provisioning closer down to the uStreamer launcher tasks because the TC358743 provisioning sets a bunch of ustreamer variables which is only put into effect by the uStreamer launcher.

This PR only writes the uStreamer Janus plugin config file after we have determined the uStreamer capture device.

Review on CodeApprove

jdeanwallace commented 1 year ago

@mtlynch - I'm requesting a review from you because I see you're working on https://github.com/tiny-pilot/tinypilot/pull/1554, which touches the same files (and I suspect would also fix https://github.com/tiny-pilot/tinypilot/issues/1567)

jdeanwallace commented 1 year ago
Automated comment from CodeApprove ➜

⏳ @mtlynch please review this Pull Request