Skip to content
This repository has been archived by the owner on Apr 5, 2023. It is now read-only.

Eliminate refcount.h workaround for uStreamer compilation #192

Open
mtlynch opened this issue Mar 24, 2022 · 0 comments
Open

Eliminate refcount.h workaround for uStreamer compilation #192

mtlynch opened this issue Mar 24, 2022 · 0 comments
Labels

Comments

@mtlynch
Copy link
Contributor

mtlynch commented Mar 24, 2022

I'm not sure if this is a Janus bug or a uStreamer bug, but the uStreamer Janus plugin won't compile unless we manually fix a C #include line in a Janus header.

It looks like Janus expects clients to have the janus header directory directly included when a plugin compiles, whereas uStreamer expects to only have the include/ dir included and expects Janus headers to be under <janus/whatever.h>, which seems reasonable.

@mtlynch mtlynch added the large label Mar 24, 2022
jdeanwallace added a commit to tiny-pilot/ansible-role-ustreamer that referenced this issue Dec 2, 2022
Part of #77

I was able to successfully test the following:
- Install TinyPilot Community on Hobbyist device running Raspberry Pi OS
Buster results in [Janus
`0.9.2`](https://packages.debian.org/buster-backports/janus)
- Install TinyPilot Community on Hobbyist device running Raspberry Pi OS
Bullseye results in [Janus
`1.0.1`](https://packages.debian.org/bullseye-backports/janus)
- Update TinyPilot Community (via terminal) on Hobbyist device running
Raspberry Pi OS Buster and our custom Janus package results in [Janus
`0.9.2`](https://packages.debian.org/buster-backports/janus)
- Update TinyPilot Community (via terminal) on Hobbyist device running
Raspberry Pi OS Bullseye and our custom Janus package results in [Janus
`1.0.1`](https://packages.debian.org/bullseye-backports/janus)

<s>I'm not sure / I haven't tested how this change will affect users who
already installed our custom Janus Debian package 🤔 </s>

### Notes:

1. To build uStreamer `WITH_JANUS`, we need to expose the Janus C header
files. We do this by installing
[`janus-dev`](https://packages.debian.org/buster-backports/armhf/janus-dev/filelist).
The Janus `plugins.h` file still suffers from [this
bug](tiny-pilot/ansible-role-tinypilot#192),
so we needed to [patch it
here](https://github.com/tiny-pilot/ansible-role-ustreamer/blob/apt-get-janus/tasks/install_janus.yml#L40-L46).
2. Janus installs all plugins, transports, and loggers by default. So we
needed to [disable them
here](https://github.com/tiny-pilot/ansible-role-ustreamer/blob/apt-get-janus/templates/janus.jcfg.j2).
3. <s>Notice the [default folder for
plugins](https://github.com/tiny-pilot/ansible-role-ustreamer/blob/f42d012e11ffa3bcaaf3c349c644ebeb7a5657e5/vars/main.yml#L7),
I'm sure this would change depending on the device architecture.</s>
4. Janus spits out this warning when running on Buster:
    ```
[WARN] libnice version outdated: 0.1.14 installed, at least 0.1.15
recommended
    ```
5. With regards to testing with molecule, this repo specifies the
correct ustreamer version (v4 or v5) based on the debian image. This
almost guarantees ustreamer to build successfully `WITH_JANUS=1`.
However, when it comes to running molecule from
`ansible-role-tinypilot`, it [dynamically determines the ustreamer
version](https://github.com/tiny-pilot/ansible-role-tinypilot/blob/a6889cbbb4aa3b9bcee1cce8cccc1d7f78a36c63/tasks/ustreamer.yml#L7-L15).
This revealed some flaws in our ansible conditions. Namely around when
to install Janus audio packages, which [you also pointed out
here](tiny-pilot/ansible-role-tinypilot#240 (comment)).
To accurately distinguish between Raspberry Pi OS and Debian, we need to
use the `ansible_lsb.id` fact because `ansible_distribution=='Debian'`
for both OSs. This fact is [only defined if the `lsb-release` apt
packages has been
installed](https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_conditionals.html#conditionals-based-on-ansible-facts)
prior to ansible gathering facts. Our docker images that molecule uses
doesn't have it installed by default, so I had to [force the
`lsb-release` installation before ansible gathers
facts](https://github.com/tiny-pilot/ansible-role-ustreamer/blob/apt-get-janus/molecule/default/converge.yml#L4-L13).
I have also [created a
PR](tiny-pilot/tinypilot#1215) to install
`lsb-release` when we install TinyPilot.
6. The follow snippets come from the `janus-debian` repo:
- The [Janus C header
hack](https://github.com/tiny-pilot/ansible-role-ustreamer/blob/apt-get-janus/tasks/install_janus.yml#L25-L31)
is based on [this
snippet](https://github.com/tiny-pilot/janus-debian/blob/master/Dockerfile#L97-L101).
- The [Janus websockets transport config]() is based on [this
snippet](https://github.com/tiny-pilot/janus-debian/blob/master/Dockerfile#L114-L121).
- The [main Janus
config](https://github.com/tiny-pilot/ansible-role-ustreamer/blob/apt-get-janus/templates/janus.jcfg.j2)
is a super lean version of the [default sample
config](https://github.com/meetecho/janus-gateway/blob/v1.0.1/conf/janus.jcfg.sample.in).
    
We can archive the `janus-debian` repo after merging this PR (probably
the [`libnice-debian`
repo](https://github.com/tiny-pilot/libnice-debian) too).
7. I no longer see the value of the `janus` ansible tag. We don't use it
anywhere and there is now a few extra helper tasks (like `set_fact`,
`include_vars`) that also would need to the `janus` tag if we wanted to
run the playbook for only janus tasks. So I just [removed all janus
tags](686b253)
instead.
8. As part of the Janus installation I also [conditionally remove our
now legacy Janus package (that we
maintained)](https://github.com/tiny-pilot/ansible-role-ustreamer/blob/apt-get-janus/tasks/install_janus.yml#L2-L15).


<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/ansible-role-ustreamer/78"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant