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

Make CEA parser & decoder as plugins #5178

Closed
tykus160 opened this issue Apr 25, 2023 · 2 comments · Fixed by #5195
Closed

Make CEA parser & decoder as plugins #5178

tykus160 opened this issue Apr 25, 2023 · 2 comments · Fixed by #5195
Assignees
@tykus160
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone
v4.4

Comments

@tykus160
Copy link
Member

tykus160 commented Apr 25, 2023 •
edited
Loading

Have you read the FAQ and checked for duplicate open issues?
Yes

Is your feature request related to a problem? Please describe.

Shaka currently has plugin system, i.e. to decide in individual build shall some text parsers be included. However, it doesn't work like that for closed captions - all of related parsers etc. are attached to the core build. If someone doesn't need TS parser or captions at all, he needs to live with that.

Describe the solution you'd like

Prepare plugin registry similar to text parsers plugins:

  1. shaka.cea.ICeaParser/ICeaDecoder:
  • change to externs
  1. shaka.media.ClosedCaptionParser:
  • add static exported methods for adding, removing and getting CEA Parser/Decoder plugins
  • get proper parser by a MIME type from plugin registry
  • fallback to shaka.cea.DummyCeaParser/Decoder if can not find plugin
  1. shaka.cea.Mp4CeaParser/TsCeaParser/CeaDecoder:
  • register as a plugin (using static methods mentioned above)
  • exclude from @core

Describe alternatives you've considered

Forking and removing unneeded dependencies manually.

Additional context

I did some experiments already and I'm happy to work on that if Shaka team thinks this idea is valuable for the project.
@tykus160 tykus160 added the type: enhancement New feature or request label Apr 25, 2023
@avelad avelad added flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P3 Useful but not urgent labels Apr 25, 2023
@github-actions github-actions bot added this to the Backlog milestone Apr 25, 2023
@avelad
Copy link
Member

avelad commented Apr 25, 2023

For me it's ok!

@joeyparrish
Copy link
Member

Sounds good. Thanks!

@tykus160 tykus160 mentioned this issue Apr 27, 2023
Merged
@avelad avelad modified the milestones: Backlog, v4.4 Apr 28, 2023
joeyparrish pushed a commit that referenced this issue Apr 29, 2023
@tykus160
feat: Convert CEA parsers to plugins (#5195)
7bda65d 
Fixes #5178 

Changes included:
- converted CEA parsers to externs
- added API to register/unregister CEA parsers
- TextEngine now checks is CEA decoder registered
- excluded CEA plugin from core build
- added lcevc plugin to core build

Bundle size results (all in KB, compared to
bf4b4a5):


  | core | complete - ui | complete - ui - cea
-- | -- | -- | --
before | 246 | 473 | 473
after | 231 | 474 | 459
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jun 28, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@tykus160 tykus160

Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Milestone
v4.4
Development

Successfully merging a pull request may close this issue.

feat: Convert CEA parsers to plugins tykus160/shaka-player
3 participants
@joeyparrish @avelad @tykus160