Skip to content

Commit

Permalink
Consider object-fit when selecting playlist by player size
Browse files Browse the repository at this point in the history
So far, when `limitRenditionByPlayerDimensions` is `true`,
`simpleSelector` tried to either find a rendition with a resolution
that matches the size of the player exactly or, if that does not
exist, a rendition with the smallest resolution that has either
greater width or greater height than the player. This makes sense
since by default the video will be scaled to fit inside the media
element. So every resolution that exceeds player size in at least one
dimension will be scaled down.

Most browsers support [1] customizing this scaling behavior via the
`object-fit` CSS property [2]. If it set to `cover`, the video will
instead be scaled up if video and player aspect ratio do not match.

The previous behavior caused renditions with low resolution to be
selected for players with small width (e.g. portrait phone aspect
ratio) even when videos were then scaled up to cover the whole player.

We therefore detect if `object-fit` is set to `cover` and instead
select the smallest rendition with a resolution that exceeds player
dimensions in both width and height.

[1] https://caniuse.com/?search=object-fit
[2] https://developer.mozilla.org/de/docs/Web/CSS/object-fit
  • Loading branch information
tf committed Jan 17, 2024
1 parent 0e0aada commit caf9ba6
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
20 changes: 19 additions & 1 deletion src/playlist-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ export const comparePlaylistResolution = function(left, right) {
* Current width of the player element (should account for the device pixel ratio)
* @param {number} settings.playerHeight
* Current height of the player element (should account for the device pixel ratio)
* @param {number} settings.playerObjectFit
* Current value of the video element's object-fit CSS property. Allows taking into
* account that the video might be scaled up to cover the media element when selecting
* media playlists based on player size.
* @param {boolean} settings.limitRenditionByPlayerDimensions
* True if the player width and height should be used during the selection, false otherwise
* @param {Object} settings.playlistController
Expand All @@ -157,6 +161,7 @@ export let simpleSelector = function(settings) {
bandwidth: playerBandwidth,
playerWidth,
playerHeight,
playerObjectFit,
limitRenditionByPlayerDimensions,
playlistController
} = settings;
Expand Down Expand Up @@ -274,7 +279,18 @@ export let simpleSelector = function(settings) {
// find the smallest variant that is larger than the player
// if there is no match of exact resolution
if (!resolutionBestRep) {
resolutionPlusOneList = haveResolution.filter((rep) => rep.width > playerWidth || rep.height > playerHeight);
resolutionPlusOneList = haveResolution.filter((rep) => {
if (playerObjectFit === 'cover') {
// video will be scaled up to cover the player. We need to
// make sure rendition is at least as wide and as high as the
// player.
return rep.width > playerWidth && rep.height > playerHeight;
}

// video will be scaled down to fit inside the player soon as
// its resolution exceeds player size in at least one dimension.
return rep.width > playerWidth || rep.height > playerHeight;
});

// find all the variants have the same smallest resolution
resolutionPlusOneSmallest = resolutionPlusOneList.filter((rep) => rep.width === resolutionPlusOneList[0].width &&
Expand Down Expand Up @@ -374,6 +390,7 @@ export const lastBandwidthSelector = function() {
bandwidth: this.systemBandwidth,
playerWidth: parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10) * pixelRatio,
playerHeight: parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10) * pixelRatio,
playerObjectFit: safeGetComputedStyle(this.tech_.el(), 'objectFit'),
limitRenditionByPlayerDimensions: this.limitRenditionByPlayerDimensions,
playlistController: this.playlistController_
});
Expand Down Expand Up @@ -425,6 +442,7 @@ export const movingAverageBandwidthSelector = function(decay) {
bandwidth: average,
playerWidth: parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10) * pixelRatio,
playerHeight: parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10) * pixelRatio,
playerObjectFit: safeGetComputedStyle(this.tech_.el(), 'objectFit'),
limitRenditionByPlayerDimensions: this.limitRenditionByPlayerDimensions,
playlistController: this.playlistController_
});
Expand Down
20 changes: 19 additions & 1 deletion test/playlist-selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,25 @@ test('simpleSelector limits using resolution information when it exists', functi
playlistController: {}
});

assert.equal(selectedPlaylist, main.playlists[3], 'selected the playlist with the lowest bandwidth higher than player resolution');
assert.equal(selectedPlaylist, main.playlists[3], 'selected the playlist with the lowest bandwidth and a resolution that exceeds player size in at least one dimension');
});

test('simpleSelector can take object fit into account', function(assert) {
const main = this.vhs.playlists.main;

main.playlists = trickyPlaylists;

const selectedPlaylist = simpleSelector({
main,
bandwidth: 4194304,
playerWidth: 444,
playerHeight: 500,
playerObjectFit: 'cover',
limitRenditionByPlayerDimensions: true,
playlistController: {}
});

assert.equal(selectedPlaylist, main.playlists[2], 'selected the playlist with the lowest bandwidth and a resolution that exceeds player size in both dimensions');
});

test('simpleSelector can not limit based on resolution information', function(assert) {
Expand Down

0 comments on commit caf9ba6

Please sign in to comment.