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

feat!: Remove DOM Parser #6063

Merged
merged 26 commits into from Jan 22, 2024
Merged

Conversation

dave-nicholas
Copy link
Contributor

@dave-nicholas dave-nicholas commented Jan 9, 2024

Background:

The native DOM Parser can perform poorly on some older devices, this approach is faster on newer devices but is considerably better on older devices.
This PR replaces the usage of the DOM Parser for DASH, MSS, VTT and TTML.

The draw back of this approach that it does not include any validation at the cost of better performance.

BEGIN_COMMIT_OVERRIDE
feat: Replace native DOM Parser with a more performant one (#6063)
END_COMMIT_OVERRIDE

use xml parser

fixed getContents bug

fix NS

sorting out typing... half way I hope

fix more typings

move type

fix some more tests

tests fixed
remove ttml and vtt parser changes for now..

remove ttml and vtt parser changes for now..

remove ttml and vtt parser changes for now..
@avelad
Copy link
Collaborator

avelad commented Jan 9, 2024

@dave-nicholas Since it change the returns of some externs, i'll change it to feat!

@avelad avelad changed the title feat: Remove DOM Parser feat!: Remove DOM Parser Jan 9, 2024
@avelad avelad added type: enhancement New feature or request type: performance A performance issue priority: P2 Smaller impact or easy workaround component: DASH The issue involves the MPEG DASH manifest format component: MSS The issue involves Microsoft Smooth Streaming manifest format labels Jan 9, 2024
@avelad avelad added this to the v5.0 milestone Jan 9, 2024
lib/dash/mpd_utils.js Outdated Show resolved Hide resolved
lib/dash/content_protection.js Outdated Show resolved Hide resolved
lib/dash/content_protection.js Outdated Show resolved Hide resolved
lib/dash/content_protection.js Outdated Show resolved Hide resolved
lib/dash/content_protection.js Outdated Show resolved Hide resolved
lib/dash/content_protection.js Outdated Show resolved Hide resolved
test/dash/mpd_utils_unit.js Outdated Show resolved Hide resolved
test/mss/mss_parser_unit.js Outdated Show resolved Hide resolved
test/mss/mss_parser_unit.js Outdated Show resolved Hide resolved
test/text/ttml_text_parser_unit.js Outdated Show resolved Hide resolved
test/text/vtt_text_parser_unit.js Outdated Show resolved Hide resolved
@dave-nicholas dave-nicholas force-pushed the txml-dash-mss branch 2 times, most recently from 1214aa4 to dcaa3e3 Compare January 15, 2024 15:46
remove unecessary space..

remove unecessary space..

remove unecessary space..
Copy link
Collaborator

@avelad avelad left a comment

Choose a reason for hiding this comment

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

LGTM, you just have to resolve some minor issues and I will approve the PR. Thanks!

docs/tutorials/upgrade.md Show resolved Hide resolved
lib/dash/dash_parser.js Outdated Show resolved Hide resolved
lib/media/drm_engine.js Show resolved Hide resolved
lib/media/drm_engine.js Outdated Show resolved Hide resolved
lib/mss/content_protection.js Show resolved Hide resolved
test/dash/dash_parser_manifest_unit.js Show resolved Hide resolved
test/dash/mpd_utils_unit.js Show resolved Hide resolved
test/dash/mpd_utils_unit.js Show resolved Hide resolved
test/dash/mpd_utils_unit.js Show resolved Hide resolved
test/dash/mpd_utils_unit.js Show resolved Hide resolved
@avelad avelad added component: TTML The issue involves TTML subtitles specifically component: WebVTT The issue involves WebVTT subtitles specifically labels Jan 18, 2024
lib/dash/content_protection.js Outdated Show resolved Hide resolved
lib/dash/mpd_utils.js Outdated Show resolved Hide resolved
lib/dash/segment_base.js Outdated Show resolved Hide resolved
lib/media/drm_engine.js Outdated Show resolved Hide resolved
lib/text/ttml_text_parser.js Outdated Show resolved Hide resolved
lib/util/tXml.js Outdated Show resolved Hide resolved
lib/util/tXml.js Outdated Show resolved Hide resolved
lib/util/tXml.js Outdated Show resolved Hide resolved
lib/util/tXml.js Outdated Show resolved Hide resolved
lib/util/tXml.js Show resolved Hide resolved
@avelad avelad requested a review from theodab January 19, 2024 15:21
@avelad
Copy link
Collaborator

avelad commented Jan 19, 2024

@theodab can you review it again? Thanks!

@avelad avelad merged commit 7116a34 into shaka-project:main Jan 22, 2024
12 of 16 checks passed
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Mar 22, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: DASH The issue involves the MPEG DASH manifest format component: MSS The issue involves Microsoft Smooth Streaming manifest format component: TTML The issue involves TTML subtitles specifically component: WebVTT The issue involves WebVTT subtitles specifically priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: enhancement New feature or request type: performance A performance issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants