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

Downloader tweaks + taskbar progress bar #265

Merged
merged 14 commits into from
May 8, 2021

Conversation

Araxeus
Copy link
Collaborator

@Araxeus Araxeus commented May 8, 2021

  • Use metadata.image from song-info provider only if not sending imgUrl from front (fixes out of sync artwork)
    const songMetadata = currentMetadata.imageSrcYTPL ? // This means metadata come from ytpl.getInfo();
    {
    ...currentMetadata,
    image: cropMaxWidth(await getImage(currentMetadata.imageSrcYTPL))
    } :
    { ...nowPlayingMetadata, ...currentMetadata };
  • Gets the best artwork possible using UrlToJPG + cropMaxWidth (in downloader/utils)
    (UrlToJPG is needed because videoDetails?.thumbnails[-1] is not always presented in jpg format (webp throw errors) + can be of different sizes. so this function guarantee the highest resolution of artwork)
    (cropMaxWidth gets pixel perfect cropping for official music artwork, without it the artwork usually have just big margins filled with solid color)
  • Dodge possible thrown errors + more accurate query selector for the "start radio" button in the popup menu:
    let videoUrl = getSongMenu()
    ?.querySelector('ytmusic-menu-navigation-item-renderer.iron-selected[tabindex="0"]')
    ?.querySelector("#navigation-endpoint")
    ?.getAttribute("href");
  • Adds a taskbar progress bar when downloading single songs, this allows seeing download progress without the submenu open (already implemented for playlists)

@Araxeus Araxeus changed the title Downloader bugfix + Taskbar progress bar Downloader tweaks + taskbar progress bar May 8, 2021
Comment on lines -60 to -61
songInfo.title = data.videoDetails?.media?.song || data?.videoDetails?.title;
songInfo.artist = data.videoDetails?.media?.artist || await getArtist(win) || cleanupArtistName(data?.videoDetails?.author);
Copy link
Collaborator Author

@Araxeus Araxeus May 8, 2021

Choose a reason for hiding this comment

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

videoDetails.media can only appear in data parsed from ytpl.getInfo()
(never from XHR, I shouldn't have added it here in the first place)

Comment on lines +7 to +17
const orderedQualityList = ["maxresdefault", "hqdefault", "mqdefault", "sdddefault"];
module.exports.UrlToJPG = (imgUrl, videoId) => {
if (!imgUrl || imgUrl.includes(".jpg")) return imgUrl;
//it will almost never get further than hqdefault
for (const quality of orderedQualityList) {
if (imgUrl.includes(quality)) {
return `https://img.youtube.com/vi/${videoId}/${quality}.jpg`;
}
}
return `https://img.youtube.com/vi/${videoId}/default.jpg`;
}
Copy link
Collaborator Author

@Araxeus Araxeus May 8, 2021

Choose a reason for hiding this comment

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

this function will 99.9% of the time:
return a maxresdefault webp converted to jpg (quite frequent with newer videos)
or return the original url (is jpg)

0.1% chance (might even be 0%)
to do more than 1 iteration over qualityList (will only happen if original doesn't contains .jpg or maxresdefault)

This was mostly coded this way to 100% guarantee a jpg url output

note that this function is only applicable to thumbnails from ytpl.getInfo and not from XMLHttpRequest (song-info provider), since they are stored in a different source and in completely different formats

@@ -3,3 +3,29 @@ const electron = require("electron");
module.exports.getFolder = (customFolder) =>
customFolder || (electron.app || electron.remote.app).getPath("downloads");
module.exports.defaultMenuDownloadLabel = "Download playlist";

const orderedQualityList = ["maxresdefault", "hqdefault", "mqdefault", "sdddefault"];
module.exports.UrlToJPG = (imgUrl, videoId) => {
Copy link
Owner

Choose a reason for hiding this comment

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

nit:

Suggested change
module.exports.UrlToJPG = (imgUrl, videoId) => {
module.exports.urlToJPG = (imgUrl, videoId) => {

@th-ch
Copy link
Owner

th-ch commented May 8, 2021

Just a nit but nothing blocking, otherwise looks good, thanks for the improvement, merging! ✅

@th-ch th-ch merged commit 0a59122 into th-ch:master May 8, 2021
@Araxeus Araxeus deleted the ensure-download-from-radio-button branch May 8, 2021 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants