-
Notifications
You must be signed in to change notification settings - Fork 425
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
chore: use transmuxer debug/warn/error events to the debug log #1155
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1155 +/- ##
==========================================
- Coverage 86.48% 86.44% -0.05%
==========================================
Files 39 39
Lines 9509 9551 +42
Branches 2192 2200 +8
==========================================
+ Hits 8224 8256 +32
- Misses 1285 1295 +10
Continue to review full report at Codecov.
|
e03d46e
to
bc0236b
Compare
@@ -81,7 +81,7 @@ | |||
"lodash-compat": "^3.10.0", | |||
"nomnoml": "^0.3.0", | |||
"rollup": "^2.36.1", | |||
"rollup-plugin-worker-factory": "0.5.5", | |||
"rollup-plugin-worker-factory": "0.5.7", |
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.
Had to update rollup-plugin-worker-factory
to call onmessage
in dispatchEvent
as we use onmessage
in this scenario in tests.
@@ -415,7 +427,10 @@ const handleSegmentBytes = ({ | |||
isEndOfTimeline, | |||
endedTimelineFn, | |||
dataFn, | |||
doneFn | |||
doneFn, |
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.
Might do a refactor after this pull request, 90% of these functions don't care about these handlers, why do we have to call each one out in the parameters every single time...
src/transmuxer-worker.js
Outdated
transmuxer.on('debug', function({message}) { | ||
self.postMessage({action: 'debug', message}); | ||
}); | ||
|
||
transmuxer.on('error', function({message}) { | ||
self.postMessage({action: 'error', message}); | ||
}); | ||
|
||
transmuxer.on('warn', function({message}) { | ||
self.postMessage({action: 'warn', message}); | ||
}); |
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.
I know the mux.js commit is already in, but I think we may be better off using log
with a level and message. error
usually is a special case separate from standard log messages, and it would also allow us to only have one function all the way through the chain.
Log debug/warn/error from the transmuxer.
videojs/mux.js#391