Get rid of uStreamer capture device Ansible vars (#1554)

Resolves #1501
Resolves #1267

This change gets rid of the `ustreamer_capture_device` Ansible var
(TC358743 vs. HDMI to USB) and the config file where we stored the
result. Instead, we'll track capture device based on whether
`dtoverlay=tc358743` appears in `/boot/config.txt`.

In theory, relying on `/boot/config.txt` will cause incorrect behavior
if a user enables the TC358743 overlay even though they're using HDMI to
USB. That's unlikely to happen in practice, so I think it's worth the
tradeoff, as this assumption eliminates a **lot** of complexity.

As a side-effect of eliminating `ustreamer_capture_device`, we can
remove a lot of cruft from the bundle install file.

### Background

We originally added the "uStreamer settings file" because we needed a
way to keep track of whether the device was using a HDMI to USB dongle
(like a DIY TinyPilot) or a TC358743-based HDMI to CSI chip (like a
Voyager).

At the time of the original implementation, the uStreamer Ansible role
was independent of TinyPilot, so it couldn't write to TinyPilot
settings. So, we created a "uStreamer settings file" at
`/home/ustreamer.config.yml` that would essentially persist the setting
for whether the system was TC358743-based so that the initiator of the
Ansible role wouldn't have to re-specify it every time.

### Implementation notes

* I'm broadening the scope a little bit by tackling #1267 at the same
time, but I feel like the major cost of both those issues is the testing
to make sure everything is correct, so I might as well fix and test both
at the same time.
* With this change, the Ansible role no longer adds the
`dtoverlay=tc358743` line to `/boot/config.txt` because it's now the
user's responsibility to do that in advance if they've got a DIY
TinyPilot with an HDMI to CSI chip. For Pro, Packer will take
responsibility for adding that line.
  * The Ansible role still adds the `dtoverlay=tc358743-audio` line.
* `bundler/bundle/install` used to be responsible for creating
`/home/tinypilot/settings.yml` if it didn't already exist, but this
change moves that responsibility to `tinypilot.postinst`.
* I'm deliberately leaving a `debug` task in the Ansible role because I
think it's helpful to have the output in logs.
* This change drops the `ustreamer_persistent` var and instead always
launches ustreamer with the `--persistent` flag. The setting used to be
unofficially configurable via settings, but we're dropping that
configurability (see
https://github.com/tiny-pilot/tinypilot-pro/issues/972).

### Documentation impact

After we merge this, we need to update our
[wiki](https://github.com/tiny-pilot/tinypilot/wiki/Installation-Options#example-tc358743-hdmi-to-csi-capture-chip)
with the new TC358743 install method, which should simplify down to:

```bash
if ! grep --silent '^dtoverlay=tc358743$' /boot/config.txt; then
  echo 'dtoverlay=tc358743' | sudo tee --append /boot/config.txt
fi
```

Corresponding changes in Pro:
https://github.com/tiny-pilot/tinypilot-pro/compare/no-capture-pro?expand=1
(difference is just Packer config)

### Manual tests

I performed the following manual tests to verify we're not breaking
functionality.

- [x] Verify TinyPilot Community install with HDMI to USB dongle
- [x] Verify TinyPilot Community install with HDMI to CSI bridge
- [x] Verify TinyPilot Pro Hobbyist image
- [x] Verify TinyPilot Pro Voyager image
- [x] Verify installing TinyPilot Pro bundle on top of TinyPilot Pro
2.6.0 Voyager image
- [x] Verify installing TinyPilot Pro bundle on top of TinyPilot Pro
2.6.0 Hobbyist image

<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1554"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

CONTRIBUTING.md

@@ -140,8 +140,7 @@ adduser \
     tinypilot
 
 # Add uStreamer configuration.
-su tinypilot
-echo 'ustreamer_capture_device: tc358743' >> ~/settings.yml
+echo "dtoverlay=tc358743" | sudo tee --append /boot/config.txt
 ```
 
 ### Installing a nightly bundle

ansible-role-ustreamer/defaults/main.yml

@@ -25,10 +25,6 @@ ustreamer_quality: null
 # depending on the hardware.
 ustreamer_brightness: null
 
-# Don't re-initialize device on timeout (for example when HDMI cable
-# was disconnected). Can be either true or false.
-ustreamer_persistent: null
-
 # Use DV-timings.
 ustreamer_use_dv_timings: null
 
@@ -38,12 +34,6 @@ ustreamer_drop_same_frames: null
 # Set TCP_NODELAY to reduce buffering on video stream.
 ustreamer_tcp_nodelay: null
 
-# Presets to use for the video capture device.
-# Options are:
-#   * uvc - For a device that appears as a regular UVC video source.
-#   * tc358743 - For HDMI to CSI devices based on the Toshiba TC358743 chip.
-ustreamer_capture_device: null
-
 # To create a new EDID:
 #  1. Convert the existing EDID to binary using "edid2bin".
 #  2. Edit the binary using "AW EDID Editor v.02.00.13".

ansible-role-ustreamer/files/launch

@@ -67,9 +67,9 @@ append_arg_and_value_if_defined() {
 
 # If the given YAML path is defined in the launcher configs, append the
 # associated uStreamer command-line flag. For example, if the path is
-# ".ustreamer_persistent" and the YAML contains the line:
-#   ustreamer_persistent: true
-# Then, the launcher will append the following arg: "--persistent"
+# ".ustreamer_use_dv_timings" and the YAML contains the line:
+#   ustreamer_use_dv_timings: true
+# Then, the launcher will append the following arg: "--dv-timings"
 append_arg_if_defined() {
   local yaml_path="$1"
   local flag="$2"
@@ -85,8 +85,9 @@ append_arg_if_defined() {
   fi
 }
 
-USTREAMER_ARGS+=('--port')
-USTREAMER_ARGS+=('48001')
+# Apply hardcoded settings that are not configurable via YAML.
+USTREAMER_ARGS+=('--port' '48001')
+USTREAMER_ARGS+=('--persistent')
 
 append_arg_and_value_if_defined '.ustreamer_video_path'       '--device'
 append_arg_and_value_if_defined '.ustreamer_encoder'          '--encoder'
@@ -98,7 +99,6 @@ append_arg_and_value_if_defined '.ustreamer_quality'          '--quality'
 append_arg_and_value_if_defined '.ustreamer_drop_same_frames' '--drop-same-frames'
 append_arg_and_value_if_defined '.ustreamer_brightness'       '--brightness'
 
-append_arg_if_defined           '.ustreamer_persistent'       '--persistent'
 append_arg_if_defined           '.ustreamer_use_dv_timings'   '--dv-timings'
 append_arg_if_defined           '.ustreamer_tcp_nodelay'      '--tcp-nodelay'
 

ansible-role-ustreamer/tasks/install_launcher.yml

@@ -48,7 +48,6 @@
     - ustreamer_workers
     - ustreamer_quality
     - ustreamer_brightness
-    - ustreamer_persistent
     - ustreamer_use_dv_timings
     - ustreamer_drop_same_frames
     - ustreamer_tcp_nodelay

ansible-role-ustreamer/tasks/main.yml

@@ -10,6 +10,33 @@
          or ustreamer_h264_sink_rm != None
          or ustreamer_h264_bitrate != None)
 
+- name: check for a boot config file
+  stat:
+    path: /boot/config.txt
+  register: boot_config_result
+
+- name: save whether boot config file exists
+  set_fact:
+    boot_config_exists: "{{ boot_config_result.stat.exists }}"
+
+- name: check if TC358743 settings (HDMI to CSI, such as Voyager series) are enabled
+  # We could theoretically use the "lineinfile" module to check for the overlay,
+  # but we're moving away from Ansible, and grep makes the logic easier to read.
+  command: grep --silent '^dtoverlay=tc358743$' /boot/config.txt
+  register: tc358743_result
+  check_mode: no
+  ignore_errors: yes
+  changed_when: no
+  when: boot_config_exists | bool
+
+- name: save whether TC358743 settings are enabled
+  set_fact:
+    is_tc358743_enabled: "{{ boot_config_exists and tc358743_result.rc == 0 }}"
+
+- name: print whether TC35843 settings are enabled
+  debug:
+    var: is_tc358743_enabled
+
 - name: determine whether to install Janus
   set_fact:
     ustreamer_install_janus: "{{ ustreamer_h264_sink != None }}"
@@ -44,35 +71,18 @@
     name: ustreamer
     enabled: yes
 
-- name: check for a boot config file
-  stat:
-    path: /boot/config.txt
-  register: boot_config_result
-
-- name: save whether boot config file exists
-  set_fact:
-    boot_config_exists: "{{ boot_config_result.stat.exists }}"
-
-- name: check whether this machine has a uStreamer settings file
-  stat:
-    path: "{{ ustreamer_settings_file }}"
-  register: ustreamer_settings_file_result
-
-- name: read saved settings
-  import_tasks: check_saved_settings.yml
-  when: ustreamer_settings_file_result.stat.exists | bool
-
 - name: configure TC358743 HDMI capture chip
   import_tasks: provision_tc358743.yml
-  when: ustreamer_capture_device == 'tc358743'
+  when: is_tc358743_enabled | bool
 
-- name: save uStreamer settings file
-  template:
-    src: config.yml.j2
-    dest: "{{ ustreamer_settings_file }}"
-    owner: "{{ ustreamer_user }}"
-    group: "{{ ustreamer_group }}"
-    mode: "0644"
+# If this system does not use a TC358743 capture chip, assume defaults for a
+# MacroSilicon MS2109-based HDMI-to-USB capture dongle.
+- name: use custom settings for TC358743 chip for any facts not already defined
+  set_fact:
+    ustreamer_encoder: "{{ 'hw' if ustreamer_encoder == None else ustreamer_encoder }}"
+    ustreamer_format: "{{ 'jpeg' if ustreamer_format == None else ustreamer_format }}"
+    ustreamer_resolution: "{{ '1920x1080' if ustreamer_resolution == None else ustreamer_resolution }}"
+  when: not (is_tc358743_enabled | bool)
 
 - name: install uStreamer launcher
   import_tasks: install_launcher.yml

ansible-role-ustreamer/tasks/provision_tc358743.yml

@@ -1,17 +1,10 @@
 ---
-- name: enable TC358743 overlay in /boot/config.txt
-  lineinfile:
-    path: /boot/config.txt
-    line: "dtoverlay=tc358743"
-    insertafter: EOF
-  when: boot_config_exists | bool
-
 # Note that the tc358743-audio overlay depends on the tc358743 overlay.
 - name: enable TC358743-audio overlay in /boot/config.txt
   lineinfile:
     path: /boot/config.txt
     line: "dtoverlay=tc358743-audio"
-    insertafter: EOF
+    insertafter: "^dtoverlay=tc358743$"
   when: boot_config_exists and ustreamer_enable_audio_streaming
 
 - name: set GPU memory to 256MB in /boot/config.txt
@@ -63,6 +56,5 @@
     ustreamer_encoder: "{{ 'm2m-image' if ustreamer_encoder == None else ustreamer_encoder }}"
     ustreamer_format: "{{ 'uyvy' if ustreamer_format == None else ustreamer_format }}"
     ustreamer_workers: "{{ 3 if ustreamer_workers == None else ustreamer_workers }}"
-    ustreamer_persistent: "{{ True if ustreamer_persistent == None else ustreamer_persistent }}"
     ustreamer_use_dv_timings: "{{ True if ustreamer_use_dv_timings == None else ustreamer_use_dv_timings }}"
     ustreamer_drop_same_frames: "{{ 30 if ustreamer_drop_same_frames == None else ustreamer_drop_same_frames }}"

ansible-role-ustreamer/templates/janus.plugin.ustreamer.jcfg.j2

@@ -2,7 +2,7 @@ memsink: {
     object = "{{ ustreamer_h264_sink }}"
 }
 
-{% if ustreamer_capture_device == 'tc358743' -%}
+{% if is_tc358743_enabled -%}
 audio: {
     device = "hw:1"
     tc358743 = "/dev/video0"

ansible-role-ustreamer/vars/main.yml

@@ -7,10 +7,6 @@ ustreamer_group: ustreamer
 ustreamer_user: ustreamer
 ustreamer_dir: /opt/ustreamer
 
-# Path to file with persistent uStreamer settings to apply across role
-# runs.
-ustreamer_settings_file: /home/{{ ustreamer_user }}/config.yml
-
 ustreamer_janus_apt_suite: "{{ ansible_distribution_release }}-backports"
 
 # These variables are only used within this role.

bundler/bundle/install

@@ -11,30 +11,10 @@ set -u
 # Echo commands before executing them, by default to stderr.
 set -x
 
-# shellcheck disable=SC1091 # Don’t follow sourced script.
-. lib.sh
-
-# Temporary file for installation settings.
-INSTALL_SETTINGS_FILE="$(mktemp --suffix .yml)"
-readonly INSTALL_SETTINGS_FILE
-
-# The eventual, permanent settings files. Note, that these might not exist
-# yet before Ansible has run for the very first time.
-readonly TINYPILOT_SETTINGS_FILE='/home/tinypilot/settings.yml'
-readonly USTREAMER_SETTINGS_FILE='/home/ustreamer/config.yml'
-
 # The filename of the TinyPilot Debian package.
 TINYPILOT_DEBIAN_PACKAGE="$(ls tinypilot*.deb)"
 readonly TINYPILOT_DEBIAN_PACKAGE
 
-# Remove temporary files & directories.
-clean_up() {
-  rm -rf "${INSTALL_SETTINGS_FILE}"
-}
-
-# Always clean up before exiting.
-trap 'clean_up' EXIT
-
 # Prevent installation on the Raspberry Pi 3.
 if grep -q "^Model *: Raspberry Pi 3" /proc/cpuinfo ; then
   echo 'You are trying to install on incompatible hardware.' >&2
@@ -62,54 +42,6 @@ if grep -q "boot=overlay" /proc/cmdline ; then
   exit 1
 fi
 
-# Check if there's already a settings file with extra installation settings.
-if [[ -f "${TINYPILOT_SETTINGS_FILE}" ]]; then
-  echo "Using settings file at: ${TINYPILOT_SETTINGS_FILE}"
-  cp "${TINYPILOT_SETTINGS_FILE}" "${INSTALL_SETTINGS_FILE}"
-else
-  echo "No pre-existing settings file found at: ${TINYPILOT_SETTINGS_FILE}"
-fi
-
-# Set default installation settings
-yaml_set_if_undefined "${INSTALL_SETTINGS_FILE}" "ustreamer_persistent" "true"
-
-# Check if this system uses the TC358743 HDMI to CSI capture bridge.
-USE_TC358743_DEFAULTS=false
-if grep --silent "^ustreamer_capture_device:" "${INSTALL_SETTINGS_FILE}"; then
-  if grep --silent "^ustreamer_capture_device: tc358743$" "${INSTALL_SETTINGS_FILE}"; then
-    USE_TC358743_DEFAULTS=true
-  fi
-# Only check the existing config file if user has not set
-# ustreamer_capture_device install variable.
-elif [ -f "${USTREAMER_SETTINGS_FILE}" ] \
-     && grep --silent 'capture_device: "tc358743"' "${USTREAMER_SETTINGS_FILE}"; then
-  USE_TC358743_DEFAULTS=true
-fi
-
-# Write uStreamer settings.
-# If this system does not use a TC358743 capture chip, assume defaults for a
-# MacroSilicon MS2109-based HDMI-to-USB capture dongle. The TC358743 defaults
-# are provided in the uStreamer Ansible role.
-# TODO: we might be able to remove these defaults from here.
-# See https://github.com/tiny-pilot/tinypilot/issues/1267
-if ! "${USE_TC358743_DEFAULTS}"; then
-  yaml_set_if_undefined "${INSTALL_SETTINGS_FILE}" "ustreamer_encoder" "hw"
-  yaml_set_if_undefined "${INSTALL_SETTINGS_FILE}" "ustreamer_format" "jpeg"
-  yaml_set_if_undefined "${INSTALL_SETTINGS_FILE}" "ustreamer_resolution" "1920x1080"
-fi
-
-# Workaround to remove settings that are not user-configurable to avoid
-# inconsistent config. We can remove this on TinyPilot's next checkpoint
-# release.
-# https://github.com/tiny-pilot/tinypilot-pro/issues/978
-sed \
-  --in-place \
-  --expression '/ustreamer_port/d' \
-  "${INSTALL_SETTINGS_FILE}"
-
-echo "Final install settings:"
-cat "${INSTALL_SETTINGS_FILE}"
-
 # Bootstrap environment for installation.
 apt-get update --allow-releaseinfo-change-suite
 apt-get install -y \
@@ -128,20 +60,25 @@ python3 -m venv venv
 pip install "pip>=21.3.1"
 pip install -r requirements.txt
 
+# If there's an existing TinyPilot settings file, pass it to Ansible to override
+# default vars.
+readonly TINYPILOT_SETTINGS_FILE='/home/tinypilot/settings.yml'
+EXTRA_PLAYBOOK_FLAGS=()
+# Check that TinyPilot settings file exists and is non-empty.
+if [[ -s "${TINYPILOT_SETTINGS_FILE}" ]]; then
+  EXTRA_PLAYBOOK_FLAGS+=('--extra-vars' "@${TINYPILOT_SETTINGS_FILE}")
+
+  echo "Final install settings:"
+  cat "${TINYPILOT_SETTINGS_FILE}"
+fi
+readonly EXTRA_PLAYBOOK_FLAGS
+
 # Run Ansible.
 ansible-playbook \
   --inventory localhost, \
   install.yml \
-  --extra-vars "@${INSTALL_SETTINGS_FILE}"
+  "${EXTRA_PLAYBOOK_FLAGS[@]}"
 
 # Install TinyPilot Debian package and resolve missing dependencies.
 apt-get install -y \
   "./${TINYPILOT_DEBIAN_PACKAGE}"
-
-# Persist installation settings.
-cp "${INSTALL_SETTINGS_FILE}" "${TINYPILOT_SETTINGS_FILE}"
-chown tinypilot:tinypilot "${TINYPILOT_SETTINGS_FILE}"
-
-# Make the installation settings file world readable so that the uStreamer
-# launcher script can access it via a symbolic link.
-chmod 0644 "${TINYPILOT_SETTINGS_FILE}"