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

Nature of Code track video preview images #1645

Open
shiffman opened this issue Jul 5, 2024 · 15 comments
Open

Nature of Code track video preview images #1645

shiffman opened this issue Jul 5, 2024 · 15 comments
Assignees

Comments

@shiffman
Copy link
Member

shiffman commented Jul 5, 2024

This is a small thing but I'm adding links from natureofcode.com to here (see: nature-of-code/noc-book-2#985) and noticed that the NOC videos are just using the default site image and it would be nice to update them at some point!

image

@fturmel
Copy link
Collaborator

fturmel commented Jul 5, 2024

Non-challenge videos rarely have distinct thumbnails on the site, probably because they are never displayed on the site and only when sharing links?

We could fix this pretty easily by downloading the YouTube video thumbnails to the codebase. They have a predictable URL structure on their CDN: https://i.ytimg.com/vi/{id}/maxresdefault.jpg

ex: https://i.ytimg.com/vi/Rob0pbE7kks/maxresdefault.jpg

EDIT: BTW there are webp versions on the YT CDN too. The site expects jpg or png for the video images, which is why I went with jpg. The extension and root directory are different: https://i.ytimg.com/vi_webp/{videoId}/maxresdefault.webp

@dipamsen
Copy link
Member

dipamsen commented Jul 5, 2024

Actually, since #1310, the image should fall back to the track's cover image, if the video itself doesn't gave a thumbnail.

For example: Video from ml5 track, doesn't have any thumbnail image associated with it, falls back to ml5 track's cover image.
image

However, there is an issue with this specific video (Vector Math, and a few others from NOC), these videos actually have a thumbnail defined (probably an artifact from the design/testing phase), which is in fact the last fallback placeholder image for all videos, which is in this case being used for the preview
image

cc @fturmel

@fturmel
Copy link
Collaborator

fturmel commented Jul 5, 2024

Ah nice to know they'll fall back to the track image properly. I also noticed these placeholder thumbnails were manually copied and not the actual fallback, a bit strange.

So, I guess we have some options for the preferred behavior:

  1. Always use the track image by deleting the non-challenge video images currently in the repo
  2. Use the YouTube thumbnails for all non-challenge videos (still falls back to track images if any would be missing in the future), essentially Update non-challenge video thumbnails + automation script #1646
  3. A mix of both? We could start from option 2 and selectively delete images for the videos that should use the track image

@dipamsen
Copy link
Member

dipamsen commented Jul 5, 2024

I do wonder, if adding all these images in the repo, even though they are nowhere used on the site, only for link preview purposes is worth it ... Could we directly set the YouTube's thumbnail url as the og:image during build time, or is that a bad idea?

@fturmel
Copy link
Collaborator

fturmel commented Jul 5, 2024

I do wonder, if adding all these images in the repo, even though they are nowhere used on the site, only for link preview purposes is worth it ... Could we directly set the YouTube's thumbnail url as the og:image during build time?

That did cross my mind, but the high res thumbnail is not always available so we have to check for 404s and downgrade accordingly to find the best available match. See https://stackoverflow.com/questions/18029550/fetch-youtube-highest-thumbnail-resolution

EDIT: You still raise a valid point, and the option 1 in my previous comment would be a way around this. The cost comes mostly in repo storage and hashes to compute during build, which I think will not be very significant in the end. The images will not get recompressed or uploaded again. The initial build and deploy for these new assets was completed in 4min 16sec https://app.netlify.com/sites/codingtrain/deploys/66885b6b5b425e00088505ef

@shiffman
Copy link
Member Author

shiffman commented Jul 6, 2024

Thanks for the work on this! I'm fine either way, this isn't really a high priority for the reason @fturmel states that they aren't ever displayed on the site. But this has revealed some helpful info for what is my priority right now over here! nature-of-code/noc-book-2#985

@dipamsen
Copy link
Member

dipamsen commented Jul 6, 2024

I have created #1648 as an alternative to #1646, corresponding to "Option 1" as discussed above.

Unrelated, here is a demo of using directly the maxres yt thumbnail url as the meta image: (using an extension to preview the image) - if video defines a thumbnail image then that is used (i.e. for challenges), otherwise the youtube thumbnail is used (i.e. for track videos)

ytogimg3.mp4

@fturmel
Copy link
Collaborator

fturmel commented Jul 6, 2024

@dipamsen Nice! It would be a great approach if the maxres image was more predictably available.

Here's an example of a 720p video without the maxres, we have to fallback to a lower resolution image:
https://thecodingtrain.com/tracks/git-and-github-for-poets/git/1-fundamentals/8-pages
https://i.ytimg.com/vi/bFVtrlyH-kc/maxresdefault.jpg <- 404
https://i.ytimg.com/vi/bFVtrlyH-kc/hqdefault.jpg <- best available

@shiffman
Copy link
Member Author

shiffman commented Jul 7, 2024

Thanks for all of this! I prefer having the specific YT thumbnail for the shared image but I don't mind having a low-res one or if it's not available just using the track thumbnail. In this case would it make sense to just maxres thumb as the preview?

Another pro is that I am slowly updating all YT thumbnails in which case I wouldn't have to remember to re-run the import script whenever a thumb changes.

The con, however, is that it's nice to have a back-up / reference of all of my YT thumbnails here in this repo.

Not sure why I am having so much trouble deciding on this! Any votes?

@fturmel
Copy link
Collaborator

fturmel commented Jul 7, 2024

I have some additional info:

  • sddefault is actually larger than hqdefault, here's the whole list of what is available with dimensions:
const thumbnailResolutions = [
  'maxresdefault', // 1280x720 - may not be available for 720p videos
  'sddefault', // 640x480
  'hqdefault', // 480x360
  'mqdefault', // 320x180
  'default' // 120x90
];
  • I tested all 444 videos on the site and the sddefault image is always available. This means that referencing the image from the YT CDN directly as demo'd by @dipamsen could reliably work and might be the best/simplest approach for the NoC site as long as it's referencing sddefault resolution or lower. (I will comment in the NoC PR in a minute)

@dipamsen
Copy link
Member

dipamsen commented Jul 7, 2024

Is using a custom proxy server which returns the highest available version of the image, viable? Here's one I just created:

@fturmel
Copy link
Collaborator

fturmel commented Jul 7, 2024

Is using a custom proxy server which returns the highest available version of the image, viable? Here's one I just created:

Yes, that's definitely a possible approach. Netlify Edge Functions are just white-labeled Deno Deploy IIRC so the demo is on point. I think it's also possible to cache the response on the CDN with the proper response headers to reduce the amount of function invocations.

Could we even just turn this into a redirect service instead? HEAD requests to the YT CDN to determine the highest res available, then respond with a 301 or 302 + YT CDN URL directly instead of eating the bandwidth cost?

Some considerations:

  • It's ripe for abuse unless we set CORS response headers and make sure we only respond for actual Coding Train video ids
  • It might be more infra cost, but if we tune things right it's probably negligible - even perhaps cheaper if YouTube actually serves all of it with redirects?

@shiffman
Copy link
Member Author

shiffman commented Jul 8, 2024

Thanks for all of this discussion! I think we should (a) delete the non-challenge video images currently in the repo (which will cause the fallback to be the track image and (b) use video thumbs when available either via the YT CDN or @dipamsen's proxy server or whatever you both recommend! I think I should go ahead and merge #1648 and then we can proceed from there? What do you all think?

@fturmel
Copy link
Collaborator

fturmel commented Jul 8, 2024

Works for me! I just closed #1646.

@shiffman
Copy link
Member Author

shiffman commented Dec 2, 2024

Sorry to not have followed this closely, is this all set now?

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 a pull request may close this issue.

3 participants