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 #1731 - ensure BaseURLs are resolved properly for SegmentBase #1732

Merged
merged 3 commits into from Jan 12, 2017

Conversation

davemevans
Copy link
Contributor

@davemevans davemevans commented Jan 9, 2017

The manifest in #1731 showed up an issue where BaseURLs were not resolved properly for SegmentBase elements if they had a path in them because the path element appeared to be resolved twice. The reason for this was that SegmentBase and BaseURL manifest types were copying the BaseURL element to initialization, but still resolving the BaseURLs anyway, leading to the behaviour seen.

Further, once the BaseURL resolution had been performed at startup time for these manifest types it could not be redone so they would not be able to benefit from BaseURL redundancy (although this is probably less of an issue since they are for on-demand use).

I tested these specific changes using:

... as well as the usual suspects from the sample list.

Also tested Webm with:

and these work great but we do not have enough test material for webm IMO.

Copy link
Contributor

@dsparacio dsparacio left a comment

Choose a reason for hiding this comment

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

LGTM. THanks for refactoring some stuff into utils. Will help with testing.

@dsparacio
Copy link
Contributor

@bbcrddave Ready to merge?

@dsparacio dsparacio closed this Jan 11, 2017
@dsparacio dsparacio reopened this Jan 11, 2017
@davemevans
Copy link
Contributor Author

Thought of a couple of tests worth doing tomorrow morning.

Assuming you are ok with the changes as they are, I'll merge after that assuming all goes well and no changes needed?

@davemevans
Copy link
Contributor Author

@AkamaiDASH This is good to go 🍾

@dsparacio dsparacio merged commit b1f8f16 into Dash-Industry-Forum:development Jan 12, 2017
@davemevans davemevans deleted the 1731 branch January 12, 2017 18:15
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