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.

Sign up for GitHub

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

Jump to bottom

fix: Probe for containerless format support. #3931

Merged
merged 1 commit into from
Feb 9, 2022
Merged

fix: Probe for containerless format support. #3931

merged 1 commit into from
Feb 9, 2022

Conversation

theodab
Copy link
Contributor

@theodab theodab commented Feb 9, 2022

Issue #2337

Change-Id: I909a1c09cac270cd4127a2ddd2ce5506d6ef0f35

Description

We were not probing MediaSource for containerless format support before.
This meant that, among other things, the demo did not register the new raw AAC asset as playable.

Screenshots (optional)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)
  • This change requires a documentation update

Checklist:

  • I have signed the Google CLA https://cla.developers.google.com
  • My code follows the style guidelines of this project
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have verified my change on multiple browsers on different platforms
  • I have run ./build/all.py and the build passes
  • I have run ./build/test.py and all tests pass

@theodab
fix: Probe for containerless format support.
53e6d7c 
Issue shaka-project#2337

Change-Id: I909a1c09cac270cd4127a2ddd2ce5506d6ef0f35
@theodab theodab added component: HLS The issue involves Apple's HLS manifest format component: demo page The issue is in the demo page; does not affect production applications labels Feb 9, 2022
@theodab theodab requested a review from joeyparrish February 9, 2022 09:31
avelad
avelad reviewed Feb 9, 2022
View reviewed changes
demo/main.js
@@ -726,6 +726,9 @@ shakaDemo.Main = class {
if (asset.features.includes(shakaAssets.Feature.MP2TS)) {
mimeTypes.push('video/mp2t');
}
if (asset.features.includes(shakaAssets.Feature.CONTAINERLESS)) {
mimeTypes.push('audio/aac');
Copy link
Member

Choose a reason for hiding this comment

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

why only try audio/aac if there are other possible formats? (audio/mpeg for example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. I should probably have the mime type be a separate feature flag, then.

Copy link
Member

Choose a reason for hiding this comment

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

This is only for the purpose of filtering assets in the demo, and I have not encountered an up-to-date browser in the last several years that doesn't support AAC.

In the library, we have the parsed manifest and knowledge of the exact format/codec, so querying for support there is more accurate and detailed. At this level, though, I think checking for raw AAC support is enough to enable what containerless content we have in the demo.

And if it turns out that I'm wrong, we can always change it later. :-)

@joeyparrish
Copy link
Member

I see some failing tests due to a flaky expectation. I've already got a quick fix, so I'll put that up shortly after I merge this.

@joeyparrish joeyparrish merged commit b005a7c into shaka-project:master Feb 9, 2022
@theodab theodab deleted the probeContainerlessSupport branch February 10, 2022 05:50
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@avelad avelad avelad left review comments

@joeyparrish joeyparrish joeyparrish approved these changes

Assignees
No one assigned
Labels
component: demo page The issue is in the demo page; does not affect production applications component: HLS The issue involves Apple's HLS manifest format status: archived Archived and locked; will not be updated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@theodab @joeyparrish @avelad