Check ffmpeg version instead of checking for presence of BTBN_PATH

[madsciencetist]

Frigate (the actual python app) assumes that if /usr/lib/btbn-ffmpeg exists, ffmpeg is version 5, and if not, ffmpeg is version 4. This assumption matches the provided Docker builds, but is a brittle coupling between Dockerfile and app - for example, it can break if the user follows the docs that suggest a custom ffmpeg (of unspecified version) can be mounted to /usr/lib/btbn-ffmpeg.

Now the ffmpeg version is directly queried from the ffmpeg binary on the path.

[netlify]

✅ Deploy Preview for frigate-docs canceled.

Name	Link
🔨 Latest commit	6bf05a8
🔍 Latest deploy log	https://app.netlify.com/sites/frigate-docs/deploys/64a5d34a7d1be60008d97afe

[NickM-27]

Frigate (the actual python app) assumes that if /usr/lib/btbn-ffmpeg exists, ffmpeg is version 6, and if not, ffmpeg is version 4.

that is not quite correct, frigate uses 5.1.2

[NickM-27]

looks like 59 is the current major version used though, so this should work as expected.

[mweinelt]

Would appreciate if this could be a pure python implementation, so the docker environment does not become more bespoke.

How about:

import subprocess
import re
from functools import cache
from typing import Optional


libavformat_version = rb"^libavformat\s+(?P<version>\d+)"

@cache
def get_libavformat_version() -> Optional[int]:
    try:
        response = subprocess.check_output([
            "ffmpeg", "-version"
        ])
    except FileNotFoundError:
        raise RuntimeError("No ffmpeg executable in PATH")

    if match := re.search(libavformat_version, response, re.MULTILINE):
        return int(match.group('version'))

    return -1

[NickM-27]

@mweinelt We are moving towards the opposite, the goal is that at some point parts of the code base won't even be implemented in python to improve performance.

subprocess is slow and especially in cases where it needs to be run multiple times is very inefficient.

[mweinelt]

The only thing I could offer to counter running it multiple times is cache out of the functools module. It ensures it is really only run once, even if imported multiple times. I've updated the snippet accordingly and tested it like so:

for _ in range(10):
    from ffmpeg import get_libavformat_version
    print(get_libavformat_version())

❯ python test2.py
called
60
60
60
60
60
60
60
60
60
60
60
60
60
60
60
60
60
60
60
60

[NickM-27]

It will be called fewer times with that, but will still be called multiple times in that case for frigate. Functools creates a cache that is used for that process, however Frigate has multiple processes that use this function.

In any case, setting an env will also work for other implementations / changes that may come in the future

[NickM-27]

@madsciencetist need to fix formatting and the tests

[madsciencetist]

To @mweinelt 's point though, won't doing it in the s6 service break the dev container workflow? Docs suggest invoking python -u -m frigate directly.

[NickM-27]

To @mweinelt 's point though, won't doing it in the s6 service break the dev container workflow? Docs suggest invoking python -u -m frigate directly.

yes, but it would just need to be added to https://github.com/blakeblackshear/frigate/blob/dev/docker/fake_frigate_run as well

or frigate-prepare could be added and it could be handled there

[madsciencetist]

@NickM-27 adding LIBAVFORMAT_VERSION_MAJOR=$(ffmpeg -version | grep -Po 'libavformat\W+\K\d+') to fake_frigate_run only defines the environment variable in the scope of fake_frigate_run, which just sleeps forever and is not the parent of the devcontainer terminals or launch environment

[NickM-27]

@NickM-27 adding LIBAVFORMAT_VERSION_MAJOR=$(ffmpeg -version | grep -Po 'libavformat\W+\K\d+') to fake_frigate_run only defines the environment variable in the scope of fake_frigate_run, which just sleeps forever and is not the parent of the devcontainer terminals or launch environment

just need to add export LIBAVFORMAT_VERSION_MAJOR so it is available to all services. Might make sense to create a frigate-prepare and do it there in that case

[madsciencetist]

@NickM-27 Adding export LIBAVFORMAT_VERSION_MAJOR to an s6 service makes the variable available to that service or its children, but not to siblings - it does not make it available to other services or to the vscode shell. If you open two shells and export a variable in one, it is not available in the other.

Solution: I added the env export to the devcontainer's .bashrc. In the devcontainer, it will be evaluated once per login shell via .bashrc, while in deployment, it will only be evaluated once ever, via the s6 service.

[madsciencetist]

Ugh, still doesn't work with unittests, because those are launched without going through s6 or a login shell. Best idea I have for unittests is to set the ENV in the Dockerfile, based on the ffmpeg in the image, and assume that no one wants to run unittests with a custom-mounted ffmpeg.

I just don't love having three separate mechanisms for defining an env. If we're willing to drop support for runtime-mounted ffmpeg altogether, just the Dockerfile ENV would be sufficient.

Or we say it doesn't matter for unit tests and just add -e LIBAVFORMAT_VERSION_MAJOR=59 to the docker run command.

[NickM-27]

I'd say just set the environment variable in the python test setUp method. The tests aren't there to test the ffmpeg version check itself.

[madsciencetist]

setUp is insufficient because the env is accessed while importing, and the imports are before the setUp method defs.

Should I just replace os.environ["LIBAVFORMAT_VERSION_MAJOR"] with os.getenv("LIBAVFORMAT_VERSION_MAJOR", "59")? I've been avoiding doing so because it hides all the other bugs.

[NickM-27]

Yeah that is probably fine, I mean that is essentially what was done previously where we assumed if ffmpeg was installed it was version 5.0 or greater. In this case runtime checks, but if it can't get the version for whatever reason it would make sense to still default to 5.0+ behavior