-
Notifications
You must be signed in to change notification settings - Fork 719
Fix music downloads and added a basic docker compose file for quick self hosting. #705
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
base: master
Are you sure you want to change the base?
Conversation
| return f"bestaudio[ext={format}]/bestaudio/best" | ||
| # Use a more robust format string that ensures audio-only download | ||
| # and has better fallbacks | ||
| return f"bestaudio[ext={format}]/bestaudio[ext=m4a]/bestaudio[ext=mp3]/bestaudio/best[ext!=webm]/best[ext!=webm]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why best[ext!=webm] twice?
| 'format': self.format, | ||
| 'socket_timeout': 30, | ||
| 'ignore_no_formats_error': True, | ||
| 'ignore_no_formats_error': False, # Changed to False to get better error messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added in 74d07f5. Let's make sure that we're not introducing a regression
| 'cookiefile': None, # Allow cookie usage if available | ||
| 'age_limit': None, # Don't restrict by age | ||
| 'geo_bypass': True, # Attempt to bypass geographic restrictions | ||
| 'geo_bypass_country': None, # Let yt-dlp choose the best bypass method | ||
| 'extractor_retries': 3, # Retry extraction up to 3 times | ||
| 'youtube_include_dash_manifest': True, # Include DASH formats for YouTube Music | ||
| 'youtube_skip_dash_manifest': False, # Don't skip DASH manifest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason for any of these changes that I'm seeing. Setting parameters to None is redundant. geo_bypass -- the user can set if they want to, no reason for us to force it. The same goes to the other non-None parameters here. Let's leave this out
| ytdl_options = dict(self.config.YTDL_OPTIONS) | ||
| if playlist_item_limit > 0: | ||
| log.info(f'playlist limit is set. Processing only first {playlist_item_limit} entries') | ||
| ytdl_options['playlistend'] = playlist_item_limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this removed?
| @@ -0,0 +1,27 @@ | |||
| # MeTube - Self-hosted YouTube downloader with web UI | |||
| # This setup allows you to modify the code and see changes reflected in the running container | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's call the file accordingly, docker-compose.yml suggests that this is a compose file for production use. We can call it docker-compose-dev-env.yml or something like that
| - OUTPUT_TEMPLATE=%(artist,uploader)s/%(artist,uploader)s - %(title)s%(genre& [%s]|)s (%(release_date>%Y-%m-%d,upload_date>%Y-%m-%d)s).%(ext)s | ||
| - OUTPUT_TEMPLATE_PLAYLIST=%(playlist_title)s/%(artist,uploader)s - %(title)s%(genre& [%s]|)s (%(release_date>%Y-%m-%d,upload_date>%Y-%m-%d)s).%(ext)s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have other defaults in the code, no reason for changing these defaults for development purposes
No description provided.