Update and simplify recording image build #647
Conversation
misi
commented
Oct 24, 2023
misi
commented
- Use ffmpeg image from alpine not build new
- Increase -thread_queue_size to not drop frames
selenium/video/Dockerfile
Outdated
| @@ -1,21 +1,7 @@ | |||
| FROM alpine:3.12 | |||
| FROM alpine:3.18 | |||
There was a problem hiding this comment.
Let's use just alpine:3 instead.
| FROM alpine:3.12 | ||
|
|
||
| RUN apk add -U x264-libs sdl2 libxcb libbz2 xset pulseaudio-utils && \ | ||
| RUN apk add -U x264-libs sdl2 libxcb libbz2 xset pulseaudio-utils ffmpeg && \ |
There was a problem hiding this comment.
We know that ffmpeg is available as Alpine package. Could please show the size of resulting video recorder image and compare it with existing video recorder image. Our initial idea was to make it smaller.
There was a problem hiding this comment.
Yes true the size increase 132Mb from 37MB, but probably a ffmpeg alpin dist package more tested, more reliable.
It is trade off.
- docker image size
- apline pkg
- more tested distribution ffmpeg pkg
- no compile time => faster image build you prefer smaller image. I can move it separate PR or Cancel.
- no risk that ports not compile
I feel there is always a higher risk to compile own binary from source using ports, than using just a dist binary. Where dist owner make sure it works well the pkg and probably run some tests and have graduated rollout of new versions, , but these are just my 2cents.
The +size 95mb that is bigger but it is still not huge.
But I can go along if you still prefer building, but I got actually an error during the build of ffmpeg with newer alpine version.
static/chrome/Dockerfile
Outdated
| @@ -22,6 +22,8 @@ COPY entrypoint.sh / | |||
| RUN \ | |||
| chmod +x /usr/bin/chromedriver && \ | |||
| chown -R selenium:nogroup /etc/opt/chrome/policies/managed/ | |||
|
|
|||
| RUN echo "cookie-file = ~/.config/pulse/cookie" >> /etc/pulse/client.conf | |||
There was a problem hiding this comment.
Let's remove changes under static/chrome as they are not related to video recording and seem to duplicate things in #646.
There was a problem hiding this comment.
Yes sure. It was based not on master. Now rebased it and force pushed.
* remove warnings with increasing the -thread_queue_size
misi
force-pushed
the
simplify-recording
branch
from
07fd05e
to
e933f55
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #647 +/- ##
======================================
Coverage 5.59% 5.59%
======================================
Files 8 8
Lines 876 876
======================================
Hits 49 49
Misses 820 820
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@misi squashed and merged. thank you for contribution! |
