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

fix: use fallback ID if Name is undefined #3195

Conversation

eowino
Copy link
Contributor

@eowino eowino commented Mar 11, 2020

Description

This PR addresses the issue described here: #3187

@eowino
Copy link
Contributor Author

eowino commented Mar 11, 2020

@dsilhavy Please review. I'll send over the feedback agreement soon.

@dsilhavy
Copy link
Collaborator

@eowino Thanks for this PR. Following our contribution guidelines, and given this is your first PR, could you please send to me a signed copy of dash.js feedback agreement?

Thanks!

@eowino
Copy link
Contributor Author

eowino commented Mar 11, 2020

@eowino Thanks for this PR. Following our contribution guidelines, and given this is your first PR, could you please send to me a signed copy of dash.js feedback agreement?

Thanks!

@dsilhavy I have now emailed you a signed copy of dash.js feedback agreement.

Copy link
Contributor

@bbert bbert left a comment

Choose a reason for hiding this comment

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

I propose you a more compact way of doing it, by replacing lines 120 to 123 by the following ones:

const name = streamIndex.getAttribute('Name');
const type = streamIndex.getAttribute('Type');
const lang = streamIndex.getAttribute('Language');
adaptationSet.id = name || (type + '_' + lang);
adaptationSet.contentType = type;
adaptationSet.lang = lang || 'und';
adaptationSet.mimeType = mimeTypeMap[type];

If you agree with that, please commit and push, and then I'll do merge

@bbert bbert added Bug MSS Microsoft Smooth Streaming labels Mar 13, 2020
@eowino
Copy link
Contributor Author

eowino commented Mar 13, 2020

@bbert

// I went with this
const fallBackId = lang ? type + '_' + lang : type;
adaptationSet.id = name || fallBackId;

// rather than this as lang is optional
adaptationSet.id = name || (type + '_' + lang);

@bbert bbert merged commit 652a74f into Dash-Industry-Forum:development Mar 13, 2020
@bbert
Copy link
Contributor

bbert commented Mar 13, 2020

Yes I saw, good point!
My reasoning is that if name was not provided, then lang was obviously provided.
But that works fine this way.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug MSS Microsoft Smooth Streaming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants