-
Notifications
You must be signed in to change notification settings - Fork 0
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: hls-parser from service locator #50
base: main
Are you sure you want to change the base?
Conversation
Note Urls will be available only after netlify deploy. PreviewApi Reference
Demo |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
+ Coverage 63.95% 64.30% +0.35%
==========================================
Files 114 114
Lines 4830 4836 +6
Branches 633 636 +3
==========================================
+ Hits 3089 3110 +21
+ Misses 1733 1719 -14
+ Partials 8 7 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
✅ Deploy Preview for videojsdev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
size-limit report 📦
|
}); | ||
return new HlsPipeline(dependencies); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the flow is following:
user registers pipeline-loaders via pipelineLoaderFactory storage, or via separate api for worker-based player (so it is opt-in operation)
user calls load with some payload includeing mimeType
player checks and finds match for loaded mimeType and pipeline-loader-factory
player creates pipeline-loader instace via found factory and injects core deps
player passes loadPlayload to the created loader instance
loader creates parser via registered factory and injects core deps
loader loads manifest/playlist
loader parses received data
loader creates pipeline via registered factories and passes core deps and parser instance and returns this instance back to the player.
so as you can see there is a layer of dependency injections
so, we should have a static members to register hls parser factory (not instance) in the hls-pipeline-loader (not pipeline itself)
anticipated follow-up question:
why do we need pipeline-loader? can’t we just create pipeline directly in the player?
because we want separate classes for live and vod pipelines and we don’t know ahead of time which one to create. We can understand it only once we load and parse playlist/manifest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, we should have a static members to register hls parser factory (not instance) in the hls-pipeline-loader (not pipeline itself)
For clarification, do we want to expose the instance of the parser (this.parser_
) from the hls-pipeline-loader
via the service-locator
or do we want to simply expose the factory? I refactored to expose registered factory, per the feedback above, however the rfc example indicates we would want to expose the current instance of the parser instead:
const hlsParser = serviceLocator.getHlsParser();
hlsParser.parse(playlist);
Description
getHlsParser
functionality to the service locator.Specific Changes proposed
expose the full HLS parser through the a
getHlsParser
function and add tests.Requirements Checklist