Use uStreamer launcher.

[jdeanwallace]

Resolves #1249

Now that the uStreamer launcher script dynamically determines the correct uStreamer command-line options, we don't need to run our Ansible playbook to apply video settings.

This PR removes the use of the update-video-settings script and uses the backported video-service module to apply the current video settings found in /home/tinypilot/settings.yml.

Notes

1. This PR is based on @mtlynch's uStreamer launcher proof of concept PR and resolves the following missing parts:

   • Add JS logic to make sure that uStreamer/Janus is up and serving again before reloading the page
     • If we reload too early, we'll get an HTTP 502 error because uStreamer isn't listening for inbound connections yet. I've seen this in MJPEG and not in H264 mode, but I'm not sure whether it could happen in H264 as well.

   To ensure that the MJPEG stream is available, I poll the /stream endpoint until it gives us a 200 OK.

   As for the H264 stream, our connection to Janus does disconnect once the new video settings are applied (because Janus is restarted), but once we reload the page Janus is already back online. I also didn't manage to get Janus to return an error after the page has been reloaded. I do think it's still technically possible to get an error in H264 mode, but rather unlikely. If we did want to eliminate any possibility of the error, we would have to implement "automatic reconnection" logic in webrtc-video.js. Some investigation reveals that we just need to recreate the Janus instance when the client is disconnected. The janus.js library does have a reconnect method to reconnect/reclaim to an existing session, but this doesn't work if the Janus service has been restarted because Janus forgets all sessions when restarted.

   • Find a way to make video settings work in dev mode
     • It used to work through the mock script, but now we're no longer calling an external script, so we need another solution.
     • Maybe just check the debug mode in video_settings.apply?

   This is no longer an issue because applying video settings now just means restarting uStreamer/Janus, and those video services are restarted in a best effort manner (i.e., we ignore any errors that might occur) and only log any potential errors. So in dev mode, the command would silently fail and the error is logged as:

   2023-01-12 16:27:15.361 video_service   ERROR Failed to restart ustreamer: Command '['sudo', '/usr/sbin/service', 'ustreamer', 'restart']' returned non-zero exit status 1.
   2023-01-12 16:29:16.778 video_service   ERROR Failed to restart janus: Command '['sudo', '/usr/sbin/service', 'janus', 'restart']' returned non-zero exit status 1.
   

   • Dump uStreamer settings in log collection script instead of systemd settings
     • Now the systemd settings are going to be uninteresting. In collect-debug-logs, we should be dumping the contents of files in /opt/ustreamer-launcher/configs.d/

   Done. As @mtlynch mentioned, we just want to log the filenames and contents of each uStreamer config file used. For example:

   uStreamer configuration
   ==> /opt/ustreamer-launcher/configs.d/000-defaults.yml <==
   ---
   ustreamer_encoder: hw
   ustreamer_format: jpeg
   ustreamer_h264_sink: tinypilot::ustreamer::h264
   ustreamer_h264_sink_mode: 777
   ustreamer_h264_sink_rm: true
   ustreamer_interface: 127.0.0.1
   ustreamer_persistent: true
   ustreamer_port: 8001
   ustreamer_resolution: 1920x1080
   ustreamer_desired_fps: 30
   
   ==> /opt/ustreamer-launcher/configs.d/100-tinypilot.yml <==
   ustreamer_desired_fps: 26
   ustreamer_encoder: hw
   ustreamer_format: jpeg
   ustreamer_persistent: true
   ustreamer_port: 8001
   ustreamer_resolution: 1920x1080
   

2. Now that we use video_service.restart to apply video settings, our video settings module looks kind of awkward. However, it still made sense to me to keep the default video settings there.

3. We needed to make /home/tinypilot/settings.yml world readable (644) so that the uStreamer launcher could access the file via the newly created 100-tinypilot.yml symbolic link

[mtlynch]

Sorry, early note on collect-debug-logs. I'd like to avoid the merging logic and just print out filename + contents for each file.

We might get rid of yq in the future (e.g., launch-ustreamer becomes a Go app to avoid the extra dependency), and I'd like the debug log collector to minimize transformations on the logs/data it's collecting.

[jdeanwallace]

Automated comment from CodeApprove ➜

⏳ @jotaen4tinypilot please review this Pull Request

[jotaen4tinypilot]

Automated comment from CodeApprove ➜

⏳ Approval Pending (5 unresolved comments)
Approval will be granted automatically when all comments are resolved

Pretty cool how swift the video settings take effect now!

I only had some minor comments, mostly suggestions to preserve some context info (since parts of the wiring are quite implicit).

---

In: app/api.py:

> Line 310
    """Applies the current video settings found in the settings file.

I feel it would be worthwhile to briefly explain in the docstring how the video settings are applied. In contrast to before, that might not be immediately obvious anymore.

I think it’s still good to keep the “apply” abstraction (just as we do), even though we are only technically restarting. (I’d perceive the restart as implementation detail of the apply procedure.)

---

In: app/static/js/controllers.js:

> Line 316
    const response = await fetch("/stream", {

Nit: can or shall we use a then chain here instead of await, for code-style consistency with the other controllers?

---

In: app/templates/custom-elements/video-settings-dialog.html:

> Line 450
            .then(() =>

Shall we add a brief comment why we need to poll here? (Basically the explanation from the PR description.)

---

In: app/video_settings.py:

> Line 1
DEFAULT_FRAME_RATE = 30

2. Now that we use video_service.restart to apply video settings, our video settings module looks kind of awkward. However, it still made sense to me to keep the default video settings there.

Just a thought, is it still worth keeping the dedicated video_settings.py file, or would it be feasible to put the 3 constants into video_service.py?

---

In: bundler/bundle/install:

> Line 129
chmod 0644 "${TINYPILOT_SETTINGS_FILE}"

Shall we add a comment here to preserve the info from the PR description in the code? (E.g., I probably wouldn’t have guessed the reasoning without your hint.)

3. We needed to make /home/tinypilot/settings.yml world readable (644) so that the uStreamer launcher could access the file via the newly created 100-tinypilot.yml symbolic link

---

👀 @jdeanwallace it's your turn please take a look

[mtlynch]

@jdeanwallace - e2e testing on 2.5.1 hasn't started yet. If we merge this in in the next hour, I can sneak this into the 2.5.1 release before testing starts this afternoon.

If you have time to address the notes, we can merge the updated version, but what I might do is merge it as-is and pull in the review follow-ups in either 2.5.2 or just pull it into the release build without additional testing (since they're mostly non-functional changes)

[jotaen4tinypilot]

From my side it’s all just minor discussion items and I’d say the changes are good to go into master as is. (So we could follow up with improvements in another PR.)

[mtlynch]

Okay, merging now. Tracking follow-up items in #1258.