Run puppeteer and ffmpeg inside docker container #1
Conversation
🔧 (nvmrc): add .nvmrc file with Node.js version 20.16.0 ✨ (index.js): use environment variables for SITE_TO_STREAM and STREAM_DEST 🔧 (package.json): add dotenv as a dev dependency and configure it in start script
✨ (docker-compose.yml): add docker-compose.yml for multi-container setup 🔧 (index.js): add '--disable-extensions' argument to puppeteer launch options
roosnic1
requested review from
devnoname120,
OanaNst,
RudolfSchreier and
swisstxtsokol
Dockerfile
Outdated
| RUN npm ci | ||
|
|
||
| # Install ffmpeg | ||
| #RUN apt-get update && apt-get install -y ffmpeg libnss3 libatk1.0-0 libatk-bridge-2.0 |
There was a problem hiding this comment.
| #RUN apt-get update && apt-get install -y ffmpeg libnss3 libatk1.0-0 libatk-bridge-2.0 |
| - STREAM_DEST=udp://127.0.0.1:1234?pkt_size=1316 | ||
| ports: | ||
| - "1234:1234" | ||
| network_mode: host |
There was a problem hiding this comment.
Same question here! (There isn't a good emoji to express this sentiment on GitHub 😆)
…r build context ⬆️ (Dockerfile): update base image to node:lts-iron for improved stability ♻️ (Dockerfile): replace `npm ci` with `npm clean-install` for cleaner dependency installation
Dockerfile
Outdated
| lsb-release \ | ||
| wget \ | ||
| xdg-utils \ | ||
| --no-install-recommends |
There was a problem hiding this comment.
You covered 2/3 of the recommendations the Docker gives about the usage of apt-get.
Could you add && rm -rf /var/lib/apt/lists/* at the end as well?
It should do nothing except reduce image size.
| @@ -36,17 +36,13 @@ const { spawn } = require('child_process'); | |||
| '-f', 'image2pipe', | |||
| '-r', '30', | |||
There was a problem hiding this comment.
You are specifying 30 fps as a target frame rate both here and on line 73 - but on line 73, it is very implicitly encoded as the result of 1000/30.
Could you define a constant and reuse it everywhere? I am sorry if this causes weird floating point issues.
This does look very easy to overlook if the next programmer just does a "CTRL-F" for "30".
There was a problem hiding this comment.
I changed that part of the code already and will ask for a review in another PR.
| '-f', 'mpegts', | ||
| 'udp://127.0.0.1:1234?pkt_size=1316' | ||
| //'srt://listener_ip:port?pkt_size=1316' // Replace with your SRT listener details | ||
| process.env.STREAM_DEST | ||
| ]); | ||
|
|
There was a problem hiding this comment.
(This regards line 73, which GitHub won't let me select :()
- Have you considered using
setInterval? - Either way, it would be safest/cleanest to also save the result of
setTimeoutorsetIntervalin a variable which is then cleared when the function exits - otherwise I am not sure if this doesn't causecaptureFrameto continuously execute even after theffmpegprocess has exited. - Also - I am not really that familiar with the logic of what happens if frame delays start piling up.
If this becomes an issue at any point, it might make sense to use a more specific method likelodash's debounce/throttle orrequestAnimationFrame, but I leave this to your judgement.
Refactored the frame capture method from using setTimeout to setInterval for improved consistency. Also, added a console log to track the interval ID. Enhanced Dockerfile to remove apt lists after installation, reducing image size.
|
@roosnic1 Just a short update since I am going on vacation for a longer period: My comments are non-blocking, and feel free to address them in a follow-up PR and override my review should you need to merge this ASAP 🙂. 🗽😎 |
|
@roosnic1 Is this PR still relevant? |
No description provided.