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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Workaround for] OOM / memory "502 Bad Gateway" crashes w/ large videos' playback or download-to-client [SEE ALSO #37 & #53] #51

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

deldesir
Copy link
Collaborator

Calibre-Web tends to load the entire video file to RAM before playing, making it challenging when only large videos are available due to oom-killer restrictions on excessive memory usage. This PR will fix this.

Use native HTML5 video player
Play video directly using native HTML5 player
scripts/omg.yml Outdated Show resolved Hide resolved
scripts/omg.yml Outdated Show resolved Hide resolved
scripts/omg.yml Outdated Show resolved Hide resolved
@holta
Copy link
Member

holta commented Nov 28, 2023

Original problem:

Side Question: Does this allow everyone access to all books, regardless whether they have a Calibre-Web account/access/authorization?

(As an intentional or unintentional side effect?)

Question copied to its own ticket:

scripts/omg.yml Outdated Show resolved Hide resolved
@deldesir
Copy link
Collaborator Author

deldesir commented Nov 29, 2023

Downloads is now optimized with this PR. The previously 4GB video is redownloaded and it's now 210.8MB. Tested on Ubuntu 22.04.3.

Should work with most YouTube videos assuming a video+audio stream, with 720p resolution and vp9 codec exists for the URL.

@holta holta changed the title Fix OOM issue - affecting large videos playback Fix OOM issue - affecting large videos' playback Nov 30, 2023
@deldesir
Copy link
Collaborator Author

deldesir commented Dec 4, 2023

Smoke-tested together with #57 on Ubuntu 24.04 (Noble Numbat)

@holta holta changed the title Fix OOM issue - affecting large videos' playback [Workaround for] OOM / memory "502 Bad Gateway" crashes w/ large videos' playback, or download to client [SEE ALSO #37 & #53] Dec 4, 2023
@holta holta changed the title [Workaround for] OOM / memory "502 Bad Gateway" crashes w/ large videos' playback, or download to client [SEE ALSO #37 & #53] [Workaround for] OOM / memory "502 Bad Gateway" crashes w/ large videos' playback or download-to-client [SEE ALSO #37 & #53] Dec 4, 2023
if file.endswith(".webm") or file.endswith(".mp4"):
video_file = os.path.join(video_path, file)
# align with nginx path
video_file = video_file.replace(config.config_calibre_dir, "/books-direct")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aligned with iiab/iiab#3680

@holta
Copy link
Member

holta commented Dec 6, 2023

@deldesir this looks great!

  1. Is this well-tested with the latest PR Restrict NGINX (public?) access to "large" files only (e.g. .mp3, .mp4, .webm) in /library/calibre-web — for PR #51 OOM workaround #57 ?

  2. Can (A) long audiobooks and (B) long audiocasts and (C) long musical performances increasingly be Guaranteed Playable & Guaranteed Downloadable on 512 MB Rapberry Pi Zero W's and similar — without OOM ("502 Bad Gateway" out-of-memory failures) — thanks to a follow-up PR building on this one?

cps/web.py Outdated Show resolved Hide resolved
cps/web.py Outdated
for file in os.listdir(video_path):
if file.endswith(".webm") or file.endswith(".mp4"):
video_file = os.path.join(video_path, file)
# align with nginx path
Copy link
Member

@holta holta Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# align with nginx path
# align with nginx path (e.g. "books-direct") specified on Lines ~24 and ~29 of:
# https://github.com/iiab/calibre-web/blob/master/scripts/calibre-web-nginx.conf

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deldesir can you take the above suggestion seriously — or if not explain why?

In lieu of soft-coding (or posting constants on top of files, as is traditional to make software more maintainable...) please at least consider clear documentation explaining how to maintain de facto global constants.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get it. Definitely useful adding context in comments.

deldesir and others added 2 commits December 6, 2023 05:42
Co-authored-by: A Holt <[email protected]>
Co-authored-by: A Holt <[email protected]>
@holta
Copy link
Member

holta commented Dec 9, 2023

QUESTION: I'd love to understand the Root Cause better (#37 and corollary #53) — before we merge this PR and its companion PR (#57) — which together risk papering over a deeper problem?

Hopefully @deldesir can explain better the Root Cause in coming days, or at least what's understood about it so far!

BACKGROUND:

  • A "crutch" (temporary workaround) is sometimes necessary of course, to heal any injury!
  • Conversely some other "crutches" create unhealthy dependencies and long-term maintenance burdens we should avoid — e.g. if the "root cause" injury can be somewhat better understood by everybody, or at least circumscribed and documented — on the road towards fixing that properly in 2024!?

🙏 Either way let's articulate & clarify the likeliest road forward, as precisely as we can anyway. 🙏

@deldesir deldesir closed this Jul 1, 2024
@deldesir deldesir deleted the deldesir-oom-fix branch July 1, 2024 21:42
@deldesir deldesir restored the deldesir-oom-fix branch July 1, 2024 22:03
@deldesir deldesir reopened this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
2 participants