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

Adding support for segments in Period and Representation. #19

Merged
merged 9 commits into from
Feb 26, 2018
Merged

Adding support for segments in Period and Representation. #19

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
be6b06f
Adding support for segments in Period and Representation
ishita12 Feb 13, 2018
d5e4cbd
Testing for getting atmost one segment at each level
ishita12 Feb 13, 2018
a7877d5
Adding tests for checking presence of multiple segment tags at a par…
ishita12 Feb 14, 2018
59e4b48
Updating the tests
ishita12 Feb 16, 2018
705b713
Updating the tests
ishita12 Feb 20, 2018
90c0595
Changing order of attributes
ishita12 Feb 20, 2018
bd9d668
Updating shallowMerge to be a deepMerge function
ishita12 Feb 20, 2018
984c176
Updating shallowMerge to be a deepMerge function
ishita12 Feb 20, 2018
425eaf9
Adding tests for testing merge function
ishita12 Feb 23, 2018
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
27 changes: 18 additions & 9 deletions src/inheritAttributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,20 @@ export const getSegmentInformation = (adaptationSet) => {
* Callback map function
*/
export const inheritBaseUrls =
(adaptationSetAttributes, adaptationSetBaseUrls, periodAdaptationSetInfo) => (representation) => {
(adaptationSetAttributes, adaptationSetBaseUrls, adaptationSetSegmentInfo) => (representation) => {
const repBaseUrlElements = findChildren(representation, 'BaseURL');
const repBaseUrls = buildBaseUrls(adaptationSetBaseUrls, repBaseUrlElements);
const attributes = shallowMerge(adaptationSetAttributes, getAttributes(representation));
const segmentInfoFromRepresenation = getSegmentInformation(representation);
const segmentRepresentationInfo = merge(periodAdaptationSetInfo, segmentInfoFromRepresenation);
const representationSegmentInfo = getSegmentInformation(representation);

return repBaseUrls.map(baseUrl => {
return {
segmentInfo: segmentRepresentationInfo,
segmentInfo: {
list: adaptationSetSegmentInfo.list && representationSegmentInfo.list ? merge(adaptationSetSegmentInfo.list, representationSegmentInfo.list) : adaptationSetSegmentInfo.list && !representationSegmentInfo.list ? adaptationSetSegmentInfo.list : !adaptationSetSegmentInfo.list && representationSegmentInfo.list ? representationSegmentInfo.list : undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nightmare to read. I think its best we try to come at this with a different approach.

The main issue with the merge is that if the second object in the merge has a property that is undefined, it overwrites that property with undefined, so removing those properties from the object before we even get to merging should solve the issue without needing all this complex logic.

I think that we should return to what you had before this commit of just merging the full segment info objects instead of merging these properties individually.

Then, to fix the issue of a property being undefined, in getSegmentInformation, before returning the segment info object, save it to a variable and then delete properties that are undefined

const segmentInfo = {
  template,
  timeline: ...
  ....
};

Object.keys(segmentInfo).forEach(key => {
  if (!segmentInfo[key]) {
    delete segmentInfo[key];
  }
});

return segmentInfo;

template: adaptationSetSegmentInfo.template && representationSegmentInfo.template ? merge(adaptationSetSegmentInfo.template, representationSegmentInfo.template) : adaptationSetSegmentInfo.template && !representationSegmentInfo.template ? adaptationSetSegmentInfo.template : !adaptationSetSegmentInfo.template && representationSegmentInfo.template ? representationSegmentInfo.template : undefined,
base: adaptationSetSegmentInfo.base && representationSegmentInfo.base ? merge(adaptationSetSegmentInfo.base, representationSegmentInfo.base) : adaptationSetSegmentInfo.base && !representationSegmentInfo.base ? adaptationSetSegmentInfo.base : !adaptationSetSegmentInfo.base && representationSegmentInfo.base ? representationSegmentInfo.base : undefined,
timeline: adaptationSetSegmentInfo.timeline && representationSegmentInfo.timeline ? merge(adaptationSetSegmentInfo.timeline, representationSegmentInfo.timeline) : adaptationSetSegmentInfo.timeline && !representationSegmentInfo.timeline ? adaptationSetSegmentInfo.timeline : !adaptationSetSegmentInfo.timeline && representationSegmentInfo.timeline ? representationSegmentInfo.timeline : undefined
},
attributes: shallowMerge(attributes, { baseUrl })
};
});
Expand Down Expand Up @@ -170,7 +174,7 @@ export const inheritBaseUrls =
* Callback map function
*/
export const toRepresentations =
(periodAttributes, periodBaseUrls, periodInfo) => (adaptationSet) => {
(periodAttributes, periodBaseUrls, periodSegmentInfo) => (adaptationSet) => {
const adaptationSetAttributes = getAttributes(adaptationSet);
const adaptationSetBaseUrls = buildBaseUrls(periodBaseUrls,
findChildren(adaptationSet, 'BaseURL'));
Expand All @@ -181,10 +185,15 @@ export const toRepresentations =
roleAttributes);
const segmentInfo = getSegmentInformation(adaptationSet);
const representations = findChildren(adaptationSet, 'Representation');
const periodAdaptationSetInfo = merge(periodInfo, segmentInfo);
const adaptationSetSegmentInfo = {
list: periodSegmentInfo.list && segmentInfo.list ? merge(periodSegmentInfo.list, segmentInfo.list) : periodSegmentInfo.list && !segmentInfo.list ? periodSegmentInfo.list : !periodSegmentInfo.list && segmentInfo.list ? segmentInfo.list : undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same strategy as my other comment here

template: periodSegmentInfo.template && segmentInfo.template ? merge(periodSegmentInfo.template, segmentInfo.template) : periodSegmentInfo.template && !segmentInfo.template ? periodSegmentInfo.template : !periodSegmentInfo.template && segmentInfo.template ? segmentInfo.template : undefined,
base: periodSegmentInfo.base && segmentInfo.base ? merge(periodSegmentInfo.base, segmentInfo.base) : periodSegmentInfo.base && !segmentInfo.base ? periodSegmentInfo.base : !periodSegmentInfo.base && segmentInfo.base ? segmentInfo.base : undefined,
timeline: periodSegmentInfo.timeline && segmentInfo.timeline ? merge(periodSegmentInfo.timeline, segmentInfo.timeline) : periodSegmentInfo.timeline && !segmentInfo.timeline ? periodSegmentInfo.timeline : !periodSegmentInfo.timeline && segmentInfo.timeline ? segmentInfo.timeline : undefined
};

return flatten(
representations.map(inheritBaseUrls(attrs, adaptationSetBaseUrls, periodAdaptationSetInfo)));
representations.map(inheritBaseUrls(attrs, adaptationSetBaseUrls, adaptationSetSegmentInfo)));
};

/**
Expand Down Expand Up @@ -217,9 +226,9 @@ export const toAdaptationSets = (mpdAttributes, mpdBaseUrls) => (period, periodI
const periodAtt = getAttributes(period);
const periodAttributes = shallowMerge(periodAtt, { periodIndex }, mpdAttributes);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets switch the order of the arguments here to be mpdAttributes, periodAtt, { periodIndex }. Off the top of my head I'm not sure if there are any overlapping attributes between the mpd or period, but to be on the safe side, switching those around will make sure any overlapping attributes get overwritten correctly from the period.

const adaptationSets = findChildren(period, 'AdaptationSet');
const periodInfo = getSegmentInformation(period);
const periodSegmentInfo = getSegmentInformation(period);

return flatten(adaptationSets.map(toRepresentations(periodAttributes, periodBaseUrls, periodInfo)));
return flatten(adaptationSets.map(toRepresentations(periodAttributes, periodBaseUrls, periodSegmentInfo)));
};

/**
Expand Down
14 changes: 7 additions & 7 deletions src/toPlaylists.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import { segmentsFromList } from './segment/segmentList';
import { segmentsFromBase } from './segment/segmentBase';
// import merge from 'deepmerge';
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover comment

Copy link
Contributor

Choose a reason for hiding this comment

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

dont forget to remove this comment

export const generateSegments = (segmentInfo, attributes) => {
if (segmentInfo.list) {
return segmentsFromList(
shallowMerge(attributes, segmentInfo.list), segmentInfo.timeline
if (segmentInfo.template) {
return segmentsFromTemplate(
shallowMerge(attributes, segmentInfo.template),
segmentInfo.timeline
);
}
if (segmentInfo.base) {
return segmentsFromBase(shallowMerge(attributes, segmentInfo.base));
}
if (segmentInfo.template) {
return segmentsFromTemplate(
shallowMerge(attributes, segmentInfo.template),
segmentInfo.timeline
if (segmentInfo.list) {
return segmentsFromList(
shallowMerge(attributes, segmentInfo.list), segmentInfo.timeline
);
}
};
Expand Down
Loading