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

dash parser attributes and utility functionality #18

Merged
merged 6 commits into from
Oct 20, 2023

Conversation

wseymour15
Copy link
Collaborator

Mainly a PR to plugin most of my changes with Dzianis'.

* @param expectedAttributes The expected attributes based on the DASH spec
* @returns A parsed and formatted list attributes.
*/
const formatAttributes = (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a key change. I think it would be nice if we could have all of the expected attributes in a separate file and then have some logic to utilize those to parse and format the attributes here.

Copy link
Owner

@dzianis-dashkevich dzianis-dashkevich Oct 19, 2023

Choose a reason for hiding this comment

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

So basically, if some attribute has a default value - we should add it to the parsedManifest initializer since typescript will warn us because we should mark such fields as available instead of optional.

By having a separate map for attributes, we create two different sources of truth here.
I believe each Processor should keep its own list of static attribute keys, and it can also maintain its own set of required attributes...

I think we should go with the same approach we already have with hls-parser:

We can ensure that required attributes are presented by utilizing safeProcess protected method. I suggest simply re-using the same approach, and we can probably share it between 2 parsers in the feature.

CleanShot 2023-10-19 at 18 57 34@2x
CleanShot 2023-10-19 at 18 54 45@2x

parsedManifest.availabilityEndTime = Date.parse(availabilityEndTime);
}
const attributes = formatAttributes(tagInfo.tagAttributes, MPDAttributes);
sharedState.attributes = attributes;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we will eventually want these attributes to live on the Representation object in the parsed manifest. Therefore, I am just writing it to the state. I'll work on improving this in the future.

import { parseDuration, parseDate } from './utils/time';

interface Parsers {
[key: string]: (value: string) => string | number;

Choose a reason for hiding this comment

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

We can probably make it typed

private static readonly MIN_BUFFER_TIME = 'minBufferTime';
private static readonly XMLNS = 'xmlns';
private static readonly XMLNS_XSI = 'xmlns:xsi';
private static readonly XSI_SCHEMA_LOCATION = 'xsi:schemaLocation';

Choose a reason for hiding this comment

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

we can def omit this xmlns, xmls-xsi and xsi-schema-location

@wseymour15 wseymour15 merged commit 97d69d4 into main Oct 20, 2023
0 of 2 checks passed
@wseymour15 wseymour15 deleted the dash-parser-processors branch October 20, 2023 16:40
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.

2 participants