Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: vhs utils update #1036

Merged
merged 10 commits into from
Jan 14, 2021
Merged

chore: vhs utils update #1036

merged 10 commits into from
Jan 14, 2021

Conversation

brandonocasey
Copy link
Contributor

@brandonocasey brandonocasey commented Dec 16, 2020

@brandonocasey brandonocasey changed the base branch from chore/better-worker-build to main December 16, 2020 16:46
gkatsev
gkatsev previously approved these changes Dec 16, 2020
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well, just needs to wait for the vhs-utils update.

src/util/codecs.js Outdated Show resolved Hide resolved
gkatsev
gkatsev previously approved these changes Jan 11, 2021
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a test and it's working. Confirmed that with this change we will properly pick up missing codecs and be able to play them even if they are partially missing from the manifest.

gkatsev
gkatsev previously approved these changes Jan 11, 2021
gesinger
gesinger previously approved these changes Jan 11, 2021
@brandonocasey brandonocasey dismissed stale reviews from gesinger and gkatsev via 549cb71 January 12, 2021 16:43
@@ -1805,6 +1805,9 @@ QUnit.test('blacklists incompatible playlists by codec, without codec switching'
});

QUnit.test('does not blacklist incompatible codecs with codec switching', function(assert) {
const oldIsTypeSupported = window.MediaSource.isTypeSupported;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edge support ac-3, we need to mock that it doesn't support it for this test.

}
}

if (blacklistReasons.length) {
variant.excludeUntil = Infinity;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved the exclude down here so we don't do it in every if.

const videoDetails = codecs.video && parseCodecs(codecs.video)[0] || null;
const audioDetails = codecs.audio && parseCodecs(codecs.audio)[0] || null;

Object.keys(playlists).forEach((key) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loop through all playlists for sure

@@ -1940,6 +1940,9 @@ QUnit.test('blacklists fmp4 playlists by browser support', function(assert) {
});

QUnit.test('blacklists ts playlists by muxer support', function(assert) {
const oldIsTypeSupported = window.MediaSource.isTypeSupported;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again with browser codec support needing to be overridden.

@brandonocasey brandonocasey merged commit b072c93 into main Jan 14, 2021
@brandonocasey brandonocasey deleted the chore/vhs-utils-update branch January 14, 2021 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants