Bugfix: IMSC1 parsing error #4218

mtoczko · 2021-08-07T14:33:10Z

This PR will...

Adds checking for style and region elements in the getAttributeNS function.
Added style support for region.

Why is this Pull Request needed?

For TTLM files without region and style elements, the parser returned the error Failed to parse IMSC1: TypeError: Cannot read property 'hasAttributeNS' of undefined'.

Are there any points in the code the reviewer needs to double check?

No

Resolves issues:

#4117

@s-cros Can You share a test stream?

Checklist

changes have been done against master branch, and PR does not conflict
new unit / functional tests have been added (whenever applicable)
API or design changes are documented in API.md

Bumps [@babel/helper-module-imports](https://github.com/babel/babel/tree/HEAD/packages/babel-helper-module-imports) from 7.12.1 to 7.12.5. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.12.5/packages/babel-helper-module-imports) Signed-off-by: dependabot[bot] <support@github.com>

…per-module-imports-7.12.5 Bump @babel/helper-module-imports from 7.12.1 to 7.12.5

Bumps [webpack-cli](https://github.com/webpack/webpack-cli) from 4.1.0 to 4.2.0. - [Release notes](https://github.com/webpack/webpack-cli/releases) - [Changelog](https://github.com/webpack/webpack-cli/blob/master/CHANGELOG.md) - [Commits](https://github.com/webpack/webpack-cli/compare/webpack-cli@4.1.0...webpack-cli@4.2.0) Signed-off-by: dependabot[bot] <support@github.com>

…li-4.2.0 Bump webpack-cli from 4.1.0 to 4.2.0

Bumps [netlify-cli](https://github.com/netlify/cli) from 2.67.2 to 2.67.4. - [Release notes](https://github.com/netlify/cli/releases) - [Changelog](https://github.com/netlify/cli/blob/master/CHANGELOG.md) - [Commits](netlify/cli@v2.67.2...v2.67.4) Signed-off-by: dependabot[bot] <support@github.com>

…li-2.67.4 Bump netlify-cli from 2.67.2 to 2.67.4

Bumps [netlify-cli](https://github.com/netlify/cli) from 2.67.4 to 2.68.0. - [Release notes](https://github.com/netlify/cli/releases) - [Changelog](https://github.com/netlify/cli/blob/master/CHANGELOG.md) - [Commits](netlify/cli@v2.67.4...v2.68.0) Signed-off-by: dependabot[bot] <support@github.com>

…li-2.68.0 Bump netlify-cli from 2.67.4 to 2.68.0

Bumps [typescript](https://github.com/Microsoft/TypeScript) from 4.0.3 to 4.0.5. - [Release notes](https://github.com/Microsoft/TypeScript/releases) - [Commits](microsoft/TypeScript@v4.0.3...v4.0.5) Signed-off-by: dependabot[bot] <support@github.com>

…t-4.0.5 Bump typescript from 4.0.3 to 4.0.5

Bumps [eslint](https://github.com/eslint/eslint) from 7.12.1 to 7.13.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.12.1...v7.13.0) Signed-off-by: dependabot[bot] <support@github.com>

…13.0 Bump eslint from 7.12.1 to 7.13.0

Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 4.6.1 to 4.7.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v4.7.0/packages/parser) Signed-off-by: dependabot[bot] <support@github.com>

…t-eslint/parser-4.7.0 Bump @typescript-eslint/parser from 4.6.1 to 4.7.0

Bumps [eslint-plugin-standard](https://github.com/standard/eslint-plugin-standard) from 4.0.2 to 4.1.0. - [Release notes](https://github.com/standard/eslint-plugin-standard/releases) - [Commits](standard/eslint-plugin-standard@v4.0.2...v4.1.0) Signed-off-by: dependabot[bot] <support@github.com>

…ugin-standard-4.1.0 Bump eslint-plugin-standard from 4.0.2 to 4.1.0

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 4.6.1 to 4.7.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v4.7.0/packages/eslint-plugin) Signed-off-by: dependabot[bot] <support@github.com>

…t-eslint/eslint-plugin-4.7.0 Bump @typescript-eslint/eslint-plugin from 4.6.1 to 4.7.0

Bumps [@types/mocha](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/mocha) from 8.0.3 to 8.0.4. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/mocha) Signed-off-by: dependabot[bot] <support@github.com>

…ha-8.0.4 Bump @types/mocha from 8.0.3 to 8.0.4

Bumps [babel-loader](https://github.com/babel/babel-loader) from 8.1.0 to 8.2.1. - [Release notes](https://github.com/babel/babel-loader/releases) - [Changelog](https://github.com/babel/babel-loader/blob/main/CHANGELOG.md) - [Commits](babel/babel-loader@v8.1.0...v8.2.1) Signed-off-by: dependabot[bot] <support@github.com>

…der-8.2.1 Bump babel-loader from 8.1.0 to 8.2.1

Bumps [netlify-cli](https://github.com/netlify/cli) from 2.68.0 to 2.68.3. - [Release notes](https://github.com/netlify/cli/releases) - [Changelog](https://github.com/netlify/cli/blob/master/CHANGELOG.md) - [Commits](netlify/cli@v2.68.0...v2.68.3) Signed-off-by: dependabot[bot] <support@github.com>

…li-2.68.3 Bump netlify-cli from 2.68.0 to 2.68.3

Bumps [netlify-cli](https://github.com/netlify/cli) from 2.68.3 to 2.68.4. - [Release notes](https://github.com/netlify/cli/releases) - [Changelog](https://github.com/netlify/cli/blob/master/CHANGELOG.md) - [Commits](netlify/cli@v2.68.3...v2.68.4) Signed-off-by: dependabot[bot] <support@github.com>

…li-2.68.4 Bump netlify-cli from 2.68.3 to 2.68.4

Bumps [netlify-cli](https://github.com/netlify/cli) from 2.68.4 to 2.68.5. - [Release notes](https://github.com/netlify/cli/releases) - [Changelog](https://github.com/netlify/cli/blob/master/CHANGELOG.md) - [Commits](netlify/cli@v2.68.4...v2.68.5) Signed-off-by: dependabot[bot] <support@github.com>

…li-2.68.5 Bump netlify-cli from 2.68.4 to 2.68.5

Bumps [webpack-merge](https://github.com/survivejs/webpack-merge) from 5.3.0 to 5.4.0. - [Release notes](https://github.com/survivejs/webpack-merge/releases) - [Changelog](https://github.com/survivejs/webpack-merge/blob/develop/CHANGELOG.md) - [Commits](survivejs/webpack-merge@v5.3.0...v5.4.0) Signed-off-by: dependabot[bot] <support@github.com>

…erge-5.4.0 Bump webpack-merge from 5.3.0 to 5.4.0

Bumps [@babel/plugin-proposal-optional-chaining](https://github.com/babel/babel/tree/HEAD/packages/babel-plugin-proposal-optional-chaining) from 7.12.13 to 7.12.16. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.12.16/packages/babel-plugin-proposal-optional-chaining) Signed-off-by: dependabot[bot] <support@github.com>

…ugin-proposal-optional-chaining-7.12.16 build(deps-dev): bump @babel/plugin-proposal-optional-chaining from 7.12.13 to 7.12.16

Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.12.13 to 7.12.16. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.12.16/packages/babel-core) Signed-off-by: dependabot[bot] <support@github.com>

…re-7.12.16 build(deps-dev): bump @babel/core from 7.12.13 to 7.12.16

Bumps [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) from 7.12.13 to 7.12.16. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.12.16/packages/babel-preset-env) Signed-off-by: dependabot[bot] <support@github.com>

…eset-env-7.12.16 build(deps-dev): bump @babel/preset-env from 7.12.13 to 7.12.16

Bumps [@babel/preset-typescript](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-typescript) from 7.12.13 to 7.12.16. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.12.16/packages/babel-preset-typescript) Signed-off-by: dependabot[bot] <support@github.com>

…eset-typescript-7.12.16 build(deps-dev): bump @babel/preset-typescript from 7.12.13 to 7.12.16

Bumps [eslint](https://github.com/eslint/eslint) from 7.19.0 to 7.20.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.19.0...v7.20.0) Signed-off-by: dependabot[bot] <support@github.com>

….20.0 build(deps-dev): bump eslint from 7.19.0 to 7.20.0

Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 4.15.0 to 4.15.1. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v4.15.1/packages/parser) Signed-off-by: dependabot[bot] <support@github.com>

…pt-eslint/parser-4.15.1 build(deps-dev): bump @typescript-eslint/parser from 4.15.0 to 4.15.1

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 4.15.0 to 4.15.1. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v4.15.1/packages/eslint-plugin) Signed-off-by: dependabot[bot] <support@github.com>

…pt-eslint/eslint-plugin-4.15.1 build(deps-dev): bump @typescript-eslint/eslint-plugin from 4.15.0 to 4.15.1

s-cros · 2021-08-09T10:09:05Z

Hello,

@mtoczko Please find a simple IMSC1 asset VOD here : https://we.tl/t-yV8reG1qkB

Br

mtoczko · 2021-08-09T13:20:58Z

Hello,

@mtoczko Please find a simple IMSC1 asset VOD here : https://we.tl/t-yV8reG1qkB

Br

@s-cros The stream has errors. You can test here: https://deploy-preview-4218--hls-js-dev.netlify.app/demo/

mtoczko · 2021-08-23T18:42:36Z

Hi @robwalch can you review?

gkatsev · 2022-02-09T22:11:24Z

src/utils/imsc1-ttml-parser.ts

@@ -182,9 +186,20 @@ function getTtmlStyles(region, style): { [style: string]: string } {
    // 'direction',
    // 'writingMode'
  ];
+
+  const regionStyleName = region.hasAttribute('style')


I've been trying to generate some test content for imsc+hls so that it can be available for testing in the long term. One of the test files I got ends up not having a region at all, which causes this to fail. Probably should add a guard for it.

Suggested change

const regionStyleName = region.hasAttribute('style')

const regionStyleName = region && region.hasAttribute('style')

Also, with that, I think the default region styles aren't quite correct, but I think that shouldn't necessarily be part of this PR.

If you have any test content that you're able to share, that would be helpful.

Hi @gkatsev
1.m4s.zip
This segment was used for testing.
When you see the file structure, you will understand the addition of the style to the region.
I don't have test streams right now

Thanks. I've been trying to generate an mp4 file like that, but all I can find will generate it with the xml documents being in the same mdat instead of across multiple mdats (which I don't think it correct).

I generated a test source to test some IMSC in HLS here https://test-streams.mux.dev/tos_ismc/main.m3u8

Co-authored-by: Gary Katsevman <git@gkatsev.com>

dependabot bot and others added 30 commits November 4, 2020 06:49

Merge pull request #30 from mtoczko/dependabot/npm_and_yarn/babel/hel…

31db7ce

…per-module-imports-7.12.5 Bump @babel/helper-module-imports from 7.12.1 to 7.12.5

Merge pull request #31 from mtoczko/dependabot/npm_and_yarn/webpack-c…

30cbe45

…li-4.2.0 Bump webpack-cli from 4.1.0 to 4.2.0

Merge pull request #33 from mtoczko/dependabot/npm_and_yarn/netlify-c…

9e1a313

…li-2.67.4 Bump netlify-cli from 2.67.2 to 2.67.4

Merge pull request #34 from mtoczko/dependabot/npm_and_yarn/netlify-c…

2d0b82c

…li-2.68.0 Bump netlify-cli from 2.67.4 to 2.68.0

Merge pull request #35 from mtoczko/dependabot/npm_and_yarn/typescrip…

7cbc207

…t-4.0.5 Bump typescript from 4.0.3 to 4.0.5

Merge pull request #36 from mtoczko/dependabot/npm_and_yarn/eslint-7.…

c9efd4b

…13.0 Bump eslint from 7.12.1 to 7.13.0

Merge pull request #37 from mtoczko/dependabot/npm_and_yarn/typescrip…

495af4f

…t-eslint/parser-4.7.0 Bump @typescript-eslint/parser from 4.6.1 to 4.7.0

Merge pull request #38 from mtoczko/dependabot/npm_and_yarn/eslint-pl…

72d3136

…ugin-standard-4.1.0 Bump eslint-plugin-standard from 4.0.2 to 4.1.0

Merge pull request #39 from mtoczko/dependabot/npm_and_yarn/typescrip…

70283c6

…t-eslint/eslint-plugin-4.7.0 Bump @typescript-eslint/eslint-plugin from 4.6.1 to 4.7.0

Merge pull request #40 from mtoczko/dependabot/npm_and_yarn/types/moc…

4b2d3f6

…ha-8.0.4 Bump @types/mocha from 8.0.3 to 8.0.4

Merge pull request #41 from mtoczko/dependabot/npm_and_yarn/babel-loa…

c3d9255

…der-8.2.1 Bump babel-loader from 8.1.0 to 8.2.1

Merge pull request #42 from mtoczko/dependabot/npm_and_yarn/netlify-c…

3d28c71

…li-2.68.3 Bump netlify-cli from 2.68.0 to 2.68.3

Merge pull request #43 from mtoczko/dependabot/npm_and_yarn/netlify-c…

afe3391

…li-2.68.4 Bump netlify-cli from 2.68.3 to 2.68.4

Merge pull request #44 from mtoczko/dependabot/npm_and_yarn/netlify-c…

45cbc0c

…li-2.68.5 Bump netlify-cli from 2.68.4 to 2.68.5

Merge pull request #45 from mtoczko/dependabot/npm_and_yarn/webpack-m…

465ab20

…erge-5.4.0 Bump webpack-merge from 5.3.0 to 5.4.0

dependabot bot and others added 17 commits February 12, 2021 06:22

Merge pull request #176 from mtoczko/dependabot/npm_and_yarn/babel/pl…

3fb23a2

…ugin-proposal-optional-chaining-7.12.16 build(deps-dev): bump @babel/plugin-proposal-optional-chaining from 7.12.13 to 7.12.16

Merge pull request #178 from mtoczko/dependabot/npm_and_yarn/babel/co…

d43fb6a

…re-7.12.16 build(deps-dev): bump @babel/core from 7.12.13 to 7.12.16

Merge pull request #177 from mtoczko/dependabot/npm_and_yarn/babel/pr…

52ab6dd

…eset-env-7.12.16 build(deps-dev): bump @babel/preset-env from 7.12.13 to 7.12.16

Merge pull request #179 from mtoczko/dependabot/npm_and_yarn/babel/pr…

ca636c4

…eset-typescript-7.12.16 build(deps-dev): bump @babel/preset-typescript from 7.12.13 to 7.12.16

Merge pull request #180 from mtoczko/dependabot/npm_and_yarn/eslint-7…

fcc9734

….20.0 build(deps-dev): bump eslint from 7.19.0 to 7.20.0

Merge pull request #184 from mtoczko/dependabot/npm_and_yarn/typescri…

20983ad

…pt-eslint/parser-4.15.1 build(deps-dev): bump @typescript-eslint/parser from 4.15.0 to 4.15.1

Merge pull request #185 from mtoczko/dependabot/npm_and_yarn/typescri…

f5e511e

…pt-eslint/eslint-plugin-4.15.1 build(deps-dev): bump @typescript-eslint/eslint-plugin from 4.15.0 to 4.15.1

Merge branch 'master' of github.com:video-dev/hls.js

60fe8f2

fix video-dev#4117

23ee767

rollback package-lock.json

2c62bc9

Marcin Toczko and others added 2 commits August 9, 2021 15:11

Support for several "mdat" in one file

512691a

Merge branch 'master' into bugfix/imsc1-4117

50c204d

robwalch added this to the 1.2.0 milestone Dec 15, 2021

gkatsev reviewed Feb 9, 2022

View reviewed changes

mtoczko and others added 3 commits February 10, 2022 13:03

Update src/utils/imsc1-ttml-parser.ts

10d933d

Co-authored-by: Gary Katsevman <git@gkatsev.com>

Merge branch 'master' into bugfix/imsc1-4117

402cdad

fix after video-dev#4459

c33c7aa

gkatsev approved these changes Feb 10, 2022

View reviewed changes

robwalch approved these changes Mar 4, 2022

View reviewed changes

robwalch merged commit 1986431 into video-dev:master Mar 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix: IMSC1 parsing error #4218

Bugfix: IMSC1 parsing error #4218

mtoczko commented Aug 7, 2021

s-cros commented Aug 9, 2021

mtoczko commented Aug 9, 2021

mtoczko commented Aug 23, 2021

gkatsev Feb 9, 2022

gkatsev Feb 9, 2022

gkatsev Feb 9, 2022

mtoczko Feb 10, 2022

gkatsev Feb 10, 2022

gkatsev Mar 4, 2022

	const regionStyleName = region.hasAttribute('style')
	const regionStyleName = region && region.hasAttribute('style')

Bugfix: IMSC1 parsing error #4218

Bugfix: IMSC1 parsing error #4218

Conversation

mtoczko commented Aug 7, 2021

This PR will...

Why is this Pull Request needed?

Are there any points in the code the reviewer needs to double check?

Resolves issues:

Checklist

s-cros commented Aug 9, 2021

mtoczko commented Aug 9, 2021

mtoczko commented Aug 23, 2021

gkatsev Feb 9, 2022

Choose a reason for hiding this comment

gkatsev Feb 9, 2022

Choose a reason for hiding this comment

gkatsev Feb 9, 2022

Choose a reason for hiding this comment

mtoczko Feb 10, 2022

Choose a reason for hiding this comment

gkatsev Feb 10, 2022

Choose a reason for hiding this comment

gkatsev Mar 4, 2022

Choose a reason for hiding this comment