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

Ignore representations which use incompatible codecs for TrickMode #2984

Merged

Conversation

avelad
Copy link
Member

@avelad avelad commented Nov 12, 2020

Resolves: #2846

@TheModMaker
Copy link
Contributor

It's not the job of the DASH parser to filter compatible tracks, it should just parse and report the manifest. This would be better done in shaka.util.StreamUtils, which handles filtering out incompatible streams. You could just set the stream.trickModeVideo to null.

@avelad
Copy link
Member Author

avelad commented Nov 12, 2020

@TheModMaker In stream #2846 there are two representations with two different codecs (H264 and H265). Since right now there is only one trickMode track, if I choose H265 (which is the one that comes first) and then filter, I leave the user without that track. With my change this would not happen, since H264 would be chosen. I understand the parser shouldn't do this, but the change would be to add trickMode (#872) multi-track support and then do filtering (this is much more complex).

@TheModMaker
Copy link
Contributor

In that case, how about just comparing the codec of the trick mode track to match the parent? We don't support codec switching, so the stream filtering will do this anyway. Then, you should include a comment referencing #1528. You can use shaka.util.MimeUtils.getCodecBase to compare the codecs.

@avelad
Copy link
Member Author

avelad commented Nov 13, 2020

@TheModMaker I updated the implementation based on your comment. Can you check again?

const variant = manifest.variants[0];
const trickModeVideo = variant && variant.video &&
variant.video.trickModeVideo;
expect(trickModeVideo).toBeUndefined();
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest changing this to use trickModeVideo field directly. That ensures we don't get false-positive if the video is null.

Suggested change
expect(trickModeVideo).toBeUndefined();
expect(manifest.variants[0].video.trickModeVideo).toBeFalsy();

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@shaka-bot
Copy link
Collaborator

All tests passed!

@TheModMaker TheModMaker merged commit 57d11d1 into shaka-project:master Nov 13, 2020
@avelad avelad deleted the incompatible-codecs-trickmode branch November 16, 2020 06:47
joeyparrish pushed a commit that referenced this pull request Dec 14, 2020
Resolves: #2846

Backported to v2.5.x

Change-Id: Ic708cec867bc3c84f911c585a5000b21c92e55d5
joeyparrish pushed a commit that referenced this pull request Dec 14, 2020
@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.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TrickMode doesn't eliminate representations which use incompatible codecs
3 participants