Problem/Motivation
When adding a remote video in media library the standard thumbnail fetched from oEmbed.
This results in low res images when using the thumbnails while theming.
Steps to reproduce
Proposed resolution
We should get the highres thumbnails from the providers and use drupal core image styles to generate low res derivates if needed.
Remaining tasks
Decide on overall architecture. See comment 25
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #33 | 3447326.patch | 2.3 KB | jalpesh |
| #30 | xzGKYgSB.patch | 2.36 KB | relisiuol |
Issue fork drupal-3447326
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
wouter waeytens commentedComment #3
cilefen commentedComment #4
wouter waeytens commentedComment #5
cilefen commentedComment #6
isalmanhaider commented+1
#2 worked! Tested on Drupal 11.0-dev, PHP 8.3.
Good catch and a must-have fix.
By default, it pulls low resolution (480 × 360 pixels) and stores as original style.
After applying the patch, it pulls high resolution (1280 × 720 pixels) and stores as original style.
Comment #7
cilefen commentedThere is a patch here that failed testing. Please fix that and merge requests are preferred.
Comment #8
cilefen commentedAlso: It isn't clear what the screenshots in comment #6 are representing as supporting this issue being ready to commit.
Comment #9
quietone commentedFixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.
Comment #10
bessone commentedThe patch also worked on my Drupal 10.3.2 (with Thunder distro).
I haven't done any testing, just added the patch in composer.json on a demo I'm working on.
Comment #13
sahana _n commentedHi,
I created an MR Please review I tested the test cases, and now the test cases are working fine.
I would greatly appreciate your suggestions for improvement. Please let me know.
Thank you!!
Comment #14
smustgrave commentedLeft comments in MR.
Comment #15
sahana _n commentedHi @smustgrave ,
Thanks for the review.
I updated the MR. The test coverage for the provider's name is already covered.
Path: core/modules/media/tests/src/Functional/ResourceFetcherTest.php
Please let me know for other concerns. I am happy to take suggestions.
Thank you!!
Comment #16
smustgrave commentedTest coverage should be expanded for additional changes.
Comment #18
daveianoWhile it works and gets the
maxresdefaultimage for most YouTube Videos, it does not work every time. There is a chance that a video does not provide amaxresdefaultimage, just the defaulthqdefault. I guess that's something to do with the video size.I added a check that the
maxresdefaultimage is only used, when available. Otherwise, it will just return a 404.Before the change, this would leave media entities without a thumbnail at all.
Comment #19
eliechoufani commentedHello,
I had the same problème on Drupal 10.3.5
Trying the patch in #2 didn't solve the problem because the http 404 error request was overriding the response.
So I did this patch that's working for my use case, maybe it needs more testing
Comment #20
wouter waeytens commentedI can confirm that patch in #19 works :)
Comment #21
pcambraComment #22
recrit commentedThere are a few issues with the patch in #19:
Comment #23
recrit commentedPushed some fixes for better error handling. This is now working for me when Vimeo does not have a high resolution option.
Thinking about this more, I feel like it could be better handled in
\Drupal\media\Plugin\media\Source\OEmbed::getLocalThumbnailUri()so that the high resolution image is only requested once and then the local file is created. Currently, the fixes in this issue would causes the high resolution images to be request every time the resource cache is rebuilt. This seems a bit excessive to me.Comment #24
jesus_md commentedHi I have some errors that break the site when a Vimeo video is removed. So I have created a patch from Drupal 10 (Tested on 10.4.1) with Drupal 11 solution. If anyone have the same problem, try to use this patch and check if it solve the error.
If you want to replicate the error I had, just create a remote video with a fake vimeo url like this one: https://vimeo.com/3453453453453
Comment #25
dalinShouldn't the bulk of this code live within the provider rather than here?
Maybe ResourceFetcherInterface should ask the ProviderRepositoryInterface "Here's the data from the oEmbed call. Is there a URL for a higher-quality version of the thumbnail?" and it responds with a URL. Then ResourceFetcherInterface then tests to see if it can get an image from that URL, otherwise falls back to the original.
With this approach ResourceFetcherInterface doesn't need to know about how each provider's API works, and the providers don't have to replicate the work of testing to see if the higher-quality version exists.
Comment #26
dalinComment #27
dalinComment #28
relisiuol commentedFix oembed & get max thumbnail size for vimeo & youtube.
Comment #29
relisiuol commentedBetter patch for vimeo because some large thumbnails are too small.
Comment #30
relisiuol commentedComment #31
robcarrI couldn't apply the patches 28-30, but the latest MR applied. I'm assuming I need to play about with CSS on the
IFRAMEto get the larger thumbnail to render in a suitable size, rather than some magic happening out of the box.Thumbnail looks great.
Comment #33
jalpesh commentedUpdated patch with latest 11.3.8 release.