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

Error downloading specific Led Zeppelin video from YouTube [PROBLEM: videos with square brackets in their title do not download] #284

Closed
avni opened this issue Nov 17, 2024 · 10 comments
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@avni
Copy link
Member

avni commented Nov 17, 2024

Describe the bug/problem

Error downloading specific video: https://www.youtube.com/watch?v=Ly6ZhQVnVow. 2 different errors on 2 different IIABs.

  • 10/24 IIAB: ... failed to download: 500 INTERNAL SERVER ERROR
  • 9/14 IIAB: ... No video formats found!

--

To Reproduce

Steps to reproduce the behavior:

  1. Go to '/books'
  2. Click on 'Download to IIAB'
  3. Go to 'Tasks' page
  4. See error

Logfile

Environment (please complete the following information):

  • OS: MacOS 14.0
  • Calibre-Web version: 0.6.24 Beta
  • Browser: Safari 17.0
@holta holta added bug Something isn't working question Further information is requested labels Nov 17, 2024
@holta holta changed the title Error downloading specific Led Zepplin video from YouTube Error downloading specific Led Zeppelin video from YouTube Nov 17, 2024
@deldesir
Copy link
Collaborator

deldesir commented Nov 17, 2024

Both IIABs have outdated yt-dlp:

Please update your IIABs with sudo iiab-update -f

The issue is however reproducible but it is not related to IIAB Calibre-Web.
image

[Testing directly with yt-dlp...]

@deldesir
Copy link
Collaborator

image

Not a missing format issue. The error traceback is:

[2024-11-16 20:35:09,491] ERROR {cps.editbooks:326} cannot access local variable 'book_path' where it is not associated with a value
Traceback (most recent call last):
  File "/usr/local/calibre-web-py3/cps/editbooks.py", line 323, in meta
    resp = move_mediafile(requested_file, current_user_name, shelf_id, media_id)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/calibre-web-py3/cps/editbooks.py", line 313, in move_mediafile
    new_book_path = os.path.join(config.config_calibre_dir, book_path)
                                                            ^^^^^^^^^
UnboundLocalError: cannot access local variable 'book_path' where it is not associated with a value

@deldesir
Copy link
Collaborator

This issue is related to #203.

@deldesir
Copy link
Collaborator

deldesir commented Nov 17, 2024

The problem arises because when IIAB Calibre-Web is looking for the video ID, it assumes it is inside square brackets ([...]) in the filename. What happened for this specific video, the filename has two sets of square brackets. This is fixed in #285 which ensures video IDs are not always in the first set of square brackets if there are two or more of them, but in the last.

@holta
Copy link
Member

holta commented Nov 17, 2024

This is fixed in #285 which ensures video IDs are not always in the first set of square brackets if there are two or more of them, but in the last.

To be crystal clear:

  • When you say "first set of square brackets" to you mean leftmost square brackets?

Hopefully a crystal clear example can be posted to the top of PR #285. Thank you!

@deldesir
Copy link
Collaborator

  • When you say "first set of square brackets" to you mean leftmost square brackets?

Yes

Hopefully a crystal clear example can be posted to the top of PR #285. Thank you!

Done in #285 (comment)

@holta holta changed the title Error downloading specific Led Zeppelin video from YouTube Error downloading specific Led Zeppelin video from YouTube [PROBLEM: videos with square brackets in their title do not download] Nov 17, 2024
@holta
Copy link
Member

holta commented Nov 17, 2024

@avni
Copy link
Member Author

avni commented Nov 17, 2024

#285 tested ✅ The Led Zepplin video downloads fine now.

  1. TIL: video_id = original_file_name.split('[')[-1].split(']')[0]
    I did not realize that python had a shortcut for accessing the last item in a list, a la: https://stackoverflow.com/questions/21462879/in-line-split-1-what-does-the-1-in-the-square-brackets-indicate-in-pytho

Out of curiosity, @deldesir:

  1. Have you tested a file with only one bracket (either [ or ]). Does that work?
  2. Does this change need to be submitted upstream?

Closing since the specific bug reported is fixed. Thank you!

@avni avni closed this as completed Nov 17, 2024
@deldesir
Copy link
Collaborator

  1. TIL: video_id = original_file_name.split('[')[-1].split(']')[0]
    I did not realize that python had a shortcut for accessing the last item in a list, a la: https://stackoverflow.com/questions/21462879/in-line-split-1-what-does-the-1-in-the-square-brackets-indicate-in-pytho

Yes, -1 Python’s negative indexing is indeed a handy feature! It works for lists too. The link you shared explains it well, thanks for sharing it.

  1. Have you tested a file with only one bracket (either [ or ]). Does that work?

Not yet, but a filename with only one bracket, such as "example_[videoID.mp4" or "example_videoID].mp4", would cause an error (IndexError) because the code would assume the presence of both opening ([) and closing (]) brackets. To handle this scenario, I'll make a PR that adds a simple check to ensure both brackets exist before attempting to extract the ID. The logic will be updated like this:

if '[' in original_file_name and ']' in original_file_name:
    video_id = original_file_name.split('[')[-1].split(']')[0]
else:
    # Handle cases where brackets are missing
    video_id = None  # or some fallback behavior

This will make the code more robust.

  1. Does this change need to be submitted upstream?

Nope. The original Calibre-Web doesn't support video downloading

@holta
Copy link
Member

holta commented Nov 18, 2024

This will make the code more robust.

Thanks @avni & @deldesir for toughening this up — to handle all video filenames.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants