Skip to content

Commit

Permalink
Stop appending '; codecs=h264' to H.264/AVC video sources (#36361)
Browse files Browse the repository at this point in the history
  • Loading branch information
coreymasanto committed Oct 14, 2021
1 parent 73201d9 commit 5fed125
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 38 deletions.
54 changes: 17 additions & 37 deletions extensions/amp-video/0.1/test/test-video-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
const addedSource = videoEl.querySelector('source');
expect(addedSource.getAttribute('src')).to.equal('video1.mp4');
expect(addedSource.getAttribute('data-bitrate')).to.equal('700');
expect(addedSource.getAttribute('type')).to.equal(
'video/mp4; codecs=h264'
);
expect(addedSource.getAttribute('type')).to.equal('video/mp4');
});

it('should add the sources sorted by codec priority', async () => {
Expand Down Expand Up @@ -178,19 +176,12 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
await fetchCachedSources(videoEl, env.ampdoc);

const addedSources = videoEl.querySelectorAll('source');
const codecRegex = /(?<=codecs=)[A-Za-z0-9.]+/;
const srcCodec0 = addedSources[0]
.getAttribute('type')
.match(codecRegex)[0];
const srcCodec1 = addedSources[1]
.getAttribute('type')
.match(codecRegex)[0];
const srcCodec2 = addedSources[2]
.getAttribute('type')
.match(codecRegex)[0];
expect(srcCodec0).to.equal('vp09.02.30.11');
expect(srcCodec1).to.equal('h264');
expect(srcCodec2).to.equal('unknown');
const srcType0 = addedSources[0].getAttribute('type');
const srcType1 = addedSources[1].getAttribute('type');
const srcType2 = addedSources[2].getAttribute('type');
expect(srcType0).to.equal('video/mp4; codecs=vp09.02.30.11');
expect(srcType1).to.equal('video/mp4');
expect(srcType2).to.equal('video/mp4; codecs=unknown');
});

it('should add the sources sorted by bitrate, for any subset of sources whose codecs have equivalent priority', async () => {
Expand Down Expand Up @@ -268,31 +259,22 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
await fetchCachedSources(videoEl, env.ampdoc);

const addedSources = videoEl.querySelectorAll('source');
const codecRegex = /(?<=codecs=)[A-Za-z0-9.]+/;
const srcCodec0 = addedSources[0]
.getAttribute('type')
.match(codecRegex)[0];
const srcCodec1 = addedSources[1]
.getAttribute('type')
.match(codecRegex)[0];
const srcCodec2 = addedSources[2]
.getAttribute('type')
.match(codecRegex)[0];
const srcCodec3 = addedSources[3]
.getAttribute('type')
.match(codecRegex)[0];
const srcType0 = addedSources[0].getAttribute('type');
const srcType1 = addedSources[1].getAttribute('type');
const srcType2 = addedSources[2].getAttribute('type');
const srcType3 = addedSources[3].getAttribute('type');

expect(addedSources[0].getAttribute('data-bitrate')).to.equal('2000');
expect(srcCodec0).to.equal('vp09.00.15.08');
expect(srcType0).to.equal('video/mp4; codecs=vp09.00.15.08');

expect(addedSources[1].getAttribute('data-bitrate')).to.equal('1000');
expect(srcCodec1).to.equal('vp09.02.30.11');
expect(srcType1).to.equal('video/mp4; codecs=vp09.02.30.11');

expect(addedSources[2].getAttribute('data-bitrate')).to.equal('3000');
expect(srcCodec2).to.equal('h264');
expect(srcType2).to.equal('video/mp4');

expect(addedSources[3].getAttribute('data-bitrate')).to.equal('2000');
expect(srcCodec3).to.equal('h264');
expect(srcType3).to.equal('video/mp4');
});

it('should add video[src] as the last fallback source', async () => {
Expand Down Expand Up @@ -324,15 +306,13 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {

const videoEl = createVideo([{src: 'video.mp4'}]);
videoEl.setAttribute('src', 'video1.mp4');
videoEl.setAttribute('type', 'video/mp4; codecs=h264');
videoEl.setAttribute('type', 'video/mp4');

await fetchCachedSources(videoEl, env.ampdoc);

const lastSource = videoEl.querySelector('source:last-of-type');
expect(lastSource.getAttribute('src')).to.equal('video1.mp4');
expect(lastSource.getAttribute('type')).to.equal(
'video/mp4; codecs=h264'
);
expect(lastSource.getAttribute('type')).to.equal('video/mp4');
});

it('should clear the unused sources when video[src]', async () => {
Expand Down
8 changes: 7 additions & 1 deletion extensions/amp-video/0.1/video-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,13 @@ function applySourcesToVideo(videoEl, sources, maxBitrate) {
}

let type = source['type'];
type += source['codec'] ? '; codecs=' + source['codec'] : '';
// If the codec information is available, add it to the type attribute.
// We do not append H.264 codec strings because, unlike their synonymous
// AVC codec strings (e.g., "avc1.4d002a"), "h264" is not recognized as a
// playable type by the browser.
if (source['codec'] && source['codec'] !== 'h264') {
type += '; codecs=' + source['codec'];
}
const sourceEl = createElementWithAttributes(
videoEl.ownerDocument,
'source',
Expand Down

0 comments on commit 5fed125

Please sign in to comment.