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

fix: do not try to use unsupported audio #896

Merged
merged 8 commits into from
Jul 10, 2020
Merged

fix: do not try to use unsupported audio #896

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fe529a3
fix: do not try to use unsupported audio
brandonocasey Jul 8, 2020
7f463fd
fix tests and handle partial
brandonocasey Jul 8, 2020
063cf6f
add test
brandonocasey Jul 9, 2020
e206a28
use real ac3, partial and non-partial, better code reuse
brandonocasey Jul 9, 2020
824dca6
rename files, increase timeout
brandonocasey Jul 9, 2020
48e17ef
fix tests
brandonocasey Jul 9, 2020
8f09417
add todo
brandonocasey Jul 9, 2020
674e86b
support trackinfo without audio/video
brandonocasey Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/create-test-data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const getManifests = () => (fs.readdirSync(manifestsDir) || [])
.map((f) => path.resolve(manifestsDir, f));

const getSegments = () => (fs.readdirSync(segmentsDir) || [])
.filter((f) => ((/\.(ts|mp4|key|webm)/).test(path.extname(f))))
.filter((f) => ((/\.(ts|mp4|key|webm|aac|ac3)/).test(path.extname(f))))
.map((f) => path.resolve(segmentsDir, f));

const buildManifestString = function() {
Expand Down
10 changes: 10 additions & 0 deletions src/media-segment-request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,16 @@ const handleSegmentBytes = ({
return;
}

if (typeof segment.container === 'undefined') {
segment.container = detectContainerForBytes(bytesAsUint8Array);
}

if (segment.container !== 'ts' && segment.container !== 'aac') {
trackInfoFn(segment, {hasAudio: true, hasVideo: false});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we reporting audio and not video if the container type is unknown?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally this was because we were working with a bad audio container, but I guess it's possible that we could get here for bad video too. I pushed a commit that makes both hasAudio and hasVideo false here. Needed other changes further up the chain to support that though.

doneFn(null, segment, {});
return;
}

// ts or aac
transmuxAndNotify({
segment,
Expand Down
4 changes: 2 additions & 2 deletions test/master-playlist-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ QUnit.test(
// playlist
this.standardXHRResponse(this.requests.shift());
// segment
this.standardXHRResponse(this.requests.shift());
this.standardXHRResponse(this.requests.shift(), muxedSegment());
// change the source
this.player.src({
src: 'manifest/master.m3u8',
Expand Down Expand Up @@ -3100,7 +3100,7 @@ QUnit.test('switches off subtitles on subtitle errors', function(assert) {
// media
this.standardXHRResponse(this.requests.shift());
// media segment
this.standardXHRResponse(this.requests.shift());
this.standardXHRResponse(this.requests.shift(), muxedSegment());

const textTracks = this.player.textTracks();

Expand Down
Loading