From 78c898582e7e02b7cc89ff7985cc70e91a939d40 Mon Sep 17 00:00:00 2001 From: Gabriel Majoulet Date: Fri, 23 Jul 2021 19:33:41 -0400 Subject: [PATCH] Experiment to limit max video bitrate on first page of a story. (#35389) * Reduce quality of cover page video. * Unit tests. * Legacy caching tests. --- extensions/amp-video/0.1/amp-video.js | 30 +++- .../amp-video/0.1/test/test-amp-video.js | 128 ++++++++++++++++++ extensions/amp-video/0.1/video-cache.js | 15 +- 3 files changed, 169 insertions(+), 4 deletions(-) diff --git a/extensions/amp-video/0.1/amp-video.js b/extensions/amp-video/0.1/amp-video.js index d2e7f9c32c82..1a57d0f0f23b 100644 --- a/extensions/amp-video/0.1/amp-video.js +++ b/extensions/amp-video/0.1/amp-video.js @@ -25,6 +25,7 @@ import { childElement, childElementByTag, childElementsByTag, + matches, } from '#core/dom/query'; import {descendsFromStory} from '../../../src/utils/story'; import {dev, devAssert, user} from '../../../src/log'; @@ -44,6 +45,7 @@ import {getBitrateManager} from './flexible-bitrate'; import {getMode} from '../../../src/mode'; import {htmlFor} from '#core/dom/static-template'; import {installVideoManagerForDoc} from '#service/video-manager-impl'; +import {isExperimentOn} from '#experiments'; import {listen, listenOncePromise} from '../../../src/event-helper'; import {mutedOrUnmutedEvent} from '../../../src/iframe-video'; import {propagateAttributes} from '#core/dom/propagate-attributes'; @@ -280,7 +282,11 @@ export class AmpVideo extends AMP.BaseElement { this.removeCachedSources_(); // Fetch new sources from remote video cache, opted-in through the "cache" // attribute. - return fetchCachedSources(this.element, this.getAmpDoc()); + return fetchCachedSources( + this.element, + this.getAmpDoc(), + this.getMaxBitrate_() + ); } } @@ -535,7 +541,11 @@ export class AmpVideo extends AMP.BaseElement { const qualities = Object.keys(AMP_VIDEO_QUALITY_BITRATES); const origType = source.getAttribute('type'); const origSrc = source.getAttribute('amp-orig-src'); + const maxBitrate = this.getMaxBitrate_(); qualities.forEach((quality, index) => { + if (maxBitrate < AMP_VIDEO_QUALITY_BITRATES[quality]) { + return; + } const cachedSource = addParamsToUrl(source.src, { 'amp_video_quality': quality, }); @@ -661,6 +671,24 @@ export class AmpVideo extends AMP.BaseElement { return !!this.getCachedSources_().length; } + /** + * Sets a max bitrate if video is on the first page of an amp-story doc. + * @return {number} + */ + getMaxBitrate_() { + if ( + this.isManagedByPool_() && + isExperimentOn(this.win, 'amp-story-first-page-max-bitrate') && + matches(this.element, 'amp-story-page:first-of-type amp-video') + ) { + Services.performanceFor(this.win).addEnabledExperiment( + 'amp-story-first-page-max-bitrate' + ); + return 1000; + } + return Number.POSITIVE_INFINITY; + } + /** * @private */ diff --git a/extensions/amp-video/0.1/test/test-amp-video.js b/extensions/amp-video/0.1/test/test-amp-video.js index db77dd01bb26..7236116d7ec8 100644 --- a/extensions/amp-video/0.1/test/test-amp-video.js +++ b/extensions/amp-video/0.1/test/test-amp-video.js @@ -902,6 +902,134 @@ describes.realWin( ); }); }); + + describe.only('max bitrate', () => { + beforeEach(() => { + toggleExperiment(env.win, 'amp-story-first-page-max-bitrate', true); + }); + + afterEach(() => { + toggleExperiment(env.win, 'amp-story-first-page-max-bitrate', false); + }); + + it('should apply a max bitrate on first story page', async () => { + remoteSources.push({ + url: 'https://example.com/cached-video-2000.mp4', + type: 'video/mp4', + 'bitrate_kbps': 2000, + }); + remoteSources.push({ + url: 'https://example.com/cached-video-720.mp4', + type: 'video/mp4', + 'bitrate_kbps': 720, + }); + const attrs = { + src: 'https://example-com.cdn.ampproject.org/m/s/video.mp4', + type: 'video/mp4', + 'amp-orig-src': 'https://example.com/video.mp4', + cache: 'google', + layout: 'responsive', + height: '100px', + width: '100px', + }; + const storyEl = doc.createElement('amp-story'); + const storyPageEl = doc.createElement('amp-story-page'); + const v = doc.createElement('amp-video'); + for (const key in attrs) { + v.setAttribute(key, attrs[key]); + } + storyEl.appendChild(storyPageEl); + storyPageEl.appendChild(v); + doc.body.appendChild(storyEl); + await v.buildInternal(); + await v.getImpl(false); + + try { + await v.layoutCallback(); + } catch (e) {} + + const sources = v.querySelectorAll('source'); + expect(sources[0].getAttribute('src')).to.equal( + 'https://example.com/cached-video-720.mp4' + ); + }); + + it('should not apply a max bitrate on second story page', async () => { + remoteSources.push({ + url: 'https://example.com/cached-video-2000.mp4', + type: 'video/mp4', + 'bitrate_kbps': 2000, + }); + remoteSources.push({ + url: 'https://example.com/cached-video-720.mp4', + type: 'video/mp4', + 'bitrate_kbps': 720, + }); + const attrs = { + src: 'https://example-com.cdn.ampproject.org/m/s/video.mp4', + type: 'video/mp4', + 'amp-orig-src': 'https://example.com/video.mp4', + cache: 'google', + layout: 'responsive', + height: '100px', + width: '100px', + }; + const storyEl = doc.createElement('amp-story'); + const storyPageEl = doc.createElement('amp-story-page'); + const storyPageEl2 = doc.createElement('amp-story-page'); + const v = doc.createElement('amp-video'); + for (const key in attrs) { + v.setAttribute(key, attrs[key]); + } + storyEl.appendChild(storyPageEl); + storyEl.appendChild(storyPageEl2); + storyPageEl2.appendChild(v); + doc.body.appendChild(storyEl); + await v.buildInternal(); + await v.getImpl(false); + + try { + await v.layoutCallback(); + } catch (e) {} + + const sources = v.querySelectorAll('source'); + expect(sources[0].getAttribute('src')).to.equal( + 'https://example.com/cached-video-2000.mp4' + ); + }); + + it('should apply a max bitrate on first story page with legacy caching', async () => { + const attrs = { + src: 'https://example-com.cdn.ampproject.org/m/s/video.mp4', + type: 'video/mp4', + 'amp-orig-src': 'https://example.com/video.mp4', + layout: 'responsive', + height: '100px', + width: '100px', + }; + const storyEl = doc.createElement('amp-story'); + const storyPageEl = doc.createElement('amp-story-page'); + const v = doc.createElement('amp-video'); + for (const key in attrs) { + v.setAttribute(key, attrs[key]); + } + storyEl.appendChild(storyPageEl); + storyPageEl.appendChild(v); + doc.body.appendChild(storyEl); + await v.buildInternal(); + await v.getImpl(false); + + try { + await v.layoutCallback(); + } catch (e) {} + + const sources = v.querySelectorAll('source'); + expect(sources[0].getAttribute('src')).to.equal( + 'https://example-com.cdn.ampproject.org/m/s/video.mp4?amp_video_quality=medium' + ); + expect(sources[0].getAttribute('data-bitrate')).to.equal('720'); + }); + }); }); describe('blurred image placeholder', () => { diff --git a/extensions/amp-video/0.1/video-cache.js b/extensions/amp-video/0.1/video-cache.js index c0f44ee859f7..2704933cc2b5 100644 --- a/extensions/amp-video/0.1/video-cache.js +++ b/extensions/amp-video/0.1/video-cache.js @@ -32,9 +32,14 @@ import {user} from '../../../src/log'; * * @param {!Element} videoEl * @param {!AmpDoc} ampdoc + * @param {number=} maxBitrate * @return {!Promise} */ -export function fetchCachedSources(videoEl, ampdoc) { +export function fetchCachedSources( + videoEl, + ampdoc, + maxBitrate = Number.POSITIVE_INFINITY +) { const {win} = ampdoc; // Keep non cached evergreen sources for crawlers. if (Services.platformFor(win).isBot()) { @@ -66,7 +71,7 @@ export function fetchCachedSources(videoEl, ampdoc) { }) .then((response) => response.json()) .then((jsonResponse) => - applySourcesToVideo(videoEl, jsonResponse['sources']) + applySourcesToVideo(videoEl, jsonResponse['sources'], maxBitrate) ) .catch(() => { // If cache fails, video should still load properly. @@ -92,11 +97,15 @@ function selectVideoSource(videoEl) { * * @param {!Element} videoEl * @param {!Array} sources + * @param {number} maxBitrate */ -function applySourcesToVideo(videoEl, sources) { +function applySourcesToVideo(videoEl, sources, maxBitrate) { sources .sort((a, b) => a['bitrate_kbps'] - b['bitrate_kbps']) .forEach((source) => { + if (source['bitrate_kbps'] > maxBitrate) { + return; + } const sourceEl = createElementWithAttributes( videoEl.ownerDocument, 'source',