Repository navigation
fix: load CldVideoPlayer posters when aspectRatio is set - #674
PaulBoye-py wants to merge 1 commit into
Conversation
The player splits a transformation array into separate URL steps and adds aspectRatio as a step of its own, which Cloudinary rejects for poster images. When aspectRatio is set and the transformation prop isn't an array, pass the player a single transformation object. Players without aspectRatio keep their existing URLs. See cloudinary/cloudinary-video-player#1087
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| props: Pick<CldVideoPlayerProps, 'aspectRatio' | 'transformation'>, | ||
| transformation: CloudinaryVideoPlayerOptions['transformation'] | ||
| ): CloudinaryVideoPlayerOptions['transformation'] { | ||
| if ( props.aspectRatio && !Array.isArray(props.transformation) && Array.isArray(transformation) ) { |
There was a problem hiding this comment.
Did the browser comparison include a source whose aspect ratio is different from the aspectRatio prop (e.g. a vertical or 4:3 video with aspectRatio="16:9")? Combining ar into the same step as the player's c_limit might change the delivered video's dimensions compared to 7.0.0. Could you compare the video URL dimensions for that case?
There was a problem hiding this comment.
Thanks for the question, @strausr .
I compared them with a vertical video (1080×1920) and a 4:3 one (1440×1080), both with aspectRatio="16:9", running the published 7.0.0 package against this PR in a 640px-wide player. I measured the delivered files: the video with ffprobe, and the poster by downloading it.
Vertical (9:16):
aspectRatio only: video 1080×1920 in both; poster 400 in 7.0.0, 222×394 with the PR;
with cropMode="fill": video 1080×608 in both; poster 700×394 in both;
with a transformation object: video 1080×1920 in both; poster 400 in 7.0.0, 222×394 with the PR.
4:3:
aspectRatio only: video 1440×1080 in both; poster 400 in 7.0.0, 525×394 with the PR;
with cropMode="fill": video 1440×810 in both; poster 700×394 in both;
with a transformation object: video 1440×1080 in both; poster 400 in 7.0.0, 525×394 with the PR.
Video: the delivered dimensions are the same as 7.0.0 in every case, and also with a 1200px player and on a 2× screen. By default, the player only adds c_limit and the width to the poster URL, not the video URL. ar_16:9 without a crop doesn't resize the video, whether it's in its own step or combined.
Poster: it now loads, and keeps the source's shape inside a 16:9 box of the width the player requests. That width is based on the player's on-screen width and the screen's pixel density: 700 here, 1300 for the same player on a 2× screen.
One difference I found is that with cropMode="fill", if the video is narrower than that requested width, the poster is now scaled up to it, because c_fill and the width are in the same step. In 7.0.0, the separate c_limit step kept it at the source size. For example, the vertical video in a 1200px player gives 1080×608 in 7.0.0 and 1200×675 with this PR. The video itself isn't affected.
Description
With Video Player v4 (next-cloudinary 7.0.0), setting
aspectRatioonCldVideoPlayermakes the poster image fail to load (Cloudinary returns a 400). The video itself plays normally. This is listed as a known limitation in the 7.0.0 release notes, withcropMode="fill"as the workaround.Root cause:
getVideoPlayerOptions(url-loader) passes the transformation to the player as an array:[{ quality }, ...transformation].aspectRatioas one more step on its own. The poster URL then ends with anar_16:9step that has no crop, which Cloudinary rejects for images:…/poster?tx=q_auto/ar_16:9/c_limit,w_700(400).…/poster?tx=ar_16:9,c_limit,q_auto,w_700(200).Changes:
getPlayerTransformation(in theCldVideoPlayerfolder, not exported from the package). WhenaspectRatiois set and thetransformationprop isn't an array, it combines the array fromgetVideoPlayerOptionsinto one object. In every other case it returns the transformation unchanged.CldVideoPlayerpasses its options through the helper. The logic lives in a separate file so it can be unit tested without a DOM test setup.getVideoPlayerOptions) are unaffected.What changes for users:
aspectRatio: no change. Poster and video URLs are exactly the same as in 7.0.0.aspectRatio: the poster loads again. Their poster and video URLs change (one step instead of several), so Cloudinary generates them once under the new URLs. In testing, the delivered video was identical.aspectRatiowithcropMode="fill"): posters already worked and still do. Their URLs change too, and the workaround can be removed.aspectRatiowith atransformationarray: not changed by this PR (the poster still fails), because combining the user's own steps could change what they asked for. Tracked in CldVideoPlayer: poster fails with aspectRatio and a transformation array #675, which also has a tested workaround.Issue Ticket Number
Fixes the "Known limitation" in the 7.0.0 release notes. Upstream context: cloudinary-video-player#1087.
Type of change
Checklist
Testing
Unit tests (
tests/components/getPlayerTransformation.spec.js, 5 cases). They use the realgetVideoPlayerOptions, so a change in url-loader's output would show up here:aspectRatio→ transformation array unchangedaspectRatio+ transformation object → one object (withquality)aspectRatiowithout a transformation → one objectaspectRatio+cropMode+quality→ one objectaspectRatio+ transformation array → unchangedWith the fix switched off, the 3
aspectRatiocases fail. Full suite: 21/21 pass.Browser: 14
CldVideoPlayerscenarios in the Next.js 16 test app (Video Player 4.1.2), compared with 7.0.0:aspectRatio="16:9"aspectRatio+ transformation objectaspectRatio+cropMode="fill"(workaround)aspectRatio+ transformation arrayaspectRatio(sizes, poster string/object, transformation object/array, quality, player options, controls, responsive, missing asset)Videos play in all scenarios except the deliberately missing asset. No page errors.
Build: library builds; no type errors in
src/;test:app(Next.js 16 production build) passes. The helper doesn't appear in the published type declarations.