Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow control of the dockerized RTSP server #48

Open
farshidtz opened this issue Jun 29, 2022 · 5 comments
Open

Allow control of the dockerized RTSP server #48

farshidtz opened this issue Jun 29, 2022 · 5 comments
Labels
1-low priority denoting isolated changes enhancement New feature or request help wanted Extra attention is needed
Projects

Comments

@farshidtz
Copy link
Member

馃殌 Feature Request

Relevant Package [REQUIRED]

Device USB Camera and RTSP Server

Description [REQUIRED]

The Dockerfile uses aler9/rtsp-simple-server as a base:

FROM aler9/rtsp-simple-server:v0.18.4 AS rtsp

And runs the process before starting the camera service:

echo "Run rtsp-simple-server..."
/rtsp-simple-server &
echo "Run device-usb-camera..."
/device-usb-camera $@

At the same time, the device service allows configuration of the RTSP server:

[Driver]
RtspServerHostName = "localhost"
RtspTcpPort = "8554"

I'm not convinced that adding an RTSP server inside the same docker container is a good idea. Note that rtsp-simple-server is already dockerized and easily deployable as standalone: https://hub.docker.com/r/aler9/rtsp-simple-server

There are a few issues:

  • The USB Camera service config allows using an external server, but does not allow disabling the internal one. The 9-10MB for the executable gets pulled in regardless.
  • It is unclear how someone can configure and secure the internal RTSP server. Does the upstream documentation apply to the Device USB service container too? Our README doesn't make that clear.
  • Embedding the rtsp-simple-server adds maintenance burden to EdgeX. I don't think this dependency was approved by the TSC. It's been a few weeks and we are already a few versions behind.

Describe the solution you'd like

The control of the RTSP server process can be made possible for example by reading an env var in docker-entrypoint.sh to decide if the process should run.

If the RTSP server wasn't embedded, I could easily deploy the project's container together with the Device Usb Camera service container.

Describe alternatives you've considered

@farshidtz farshidtz added the enhancement New feature or request label Jun 29, 2022
@cloudxxx8
Copy link
Member

It sounds fine with me if separating RTSP server can make it simpler and maintainable.

@lenny-goodell
Copy link
Member

@ajcasagrande, thoughts?

@cloudxxx8 cloudxxx8 added this to New Issues in Device WG via automation Jun 30, 2022
@cloudxxx8 cloudxxx8 added the help wanted Extra attention is needed label Jun 30, 2022
@cloudxxx8
Copy link
Member

we don't need to modify the code
it's basically only related to the build script change- Dockerfile and docker-entrypoint.sh
users can build the image together or separately. that's why we make the RTSP server endpoint configurable in the service

[Driver]
RtspServerHostName = "localhost"
RtspTcpPort = "8554"

building them together is simpler for some users, and that's just a reference build script implementation.
users can always modify the build for their own purpose. I thought we can still use this version for the current release since it's close to the release date, and welcome the contribution to provide another reference build script implementation for the future release.

@ajcasagrande
Copy link
Contributor

I thought you were saying the aler9/rtsp-simple-server is used as the base image, but I see it is just being used to copy the binary and default configuration.

I think it does make sense to keep it included, and have a flag or something that allows you to disable it. I'd assume the server can be manually configured by volume mounting over the rtsp-simple-server.yml file with a custom one. In fact, all options specified here seem valid except for maybe the HTTP API if that port isn't being forwarded.

Side note, the way the Dockerfile is written now should be adjusted to take a build arg of the version and not be hard-coded.

How would a non-bundled one fit in with edgex-compose?

@farshidtz
Copy link
Member Author

farshidtz commented Jul 15, 2022

all options specified here seem valid except for maybe the HTTP API if that port isn't being forwarded.

Actually, neither of the ports are explicitly exposed in the Dockerfile. RTSP's port is exposed in the reference docker compose script. I think at least the RTSP (8554) port should be exposed in the Dockerfile to make it clearer that another application runs inside the same container. Edit: Added a PR for that.

Also as reported in #51, there are six other servers inside which IMO should be disabled.

@iain-anderson iain-anderson moved this from New Issues to Release Backlog in Device WG Aug 29, 2022
@iain-anderson iain-anderson moved this from Release Backlog to Previous Release Backlog in Device WG Nov 22, 2022
@iain-anderson iain-anderson moved this from Previous Release Backlog to Release Backlog in Device WG Jan 9, 2023
@lenny-goodell lenny-goodell added the 1-low priority denoting isolated changes label Feb 7, 2023
@iain-anderson iain-anderson moved this from Release Backlog to Icebox in Device WG May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-low priority denoting isolated changes enhancement New feature or request help wanted Extra attention is needed
Projects
Device WG
  
Icebox
Development

No branches or pull requests

4 participants