-
Notifications
You must be signed in to change notification settings - Fork 793
Remove ad-hoc logging in favor of a config-enabled logging like playback-watcher #1014
Conversation
Changes need to be made in |
src/segment-loader.js
Outdated
@@ -477,19 +468,21 @@ export default class SegmentLoader extends videojs.EventTarget { | |||
return null; | |||
} | |||
|
|||
this.logger_('cB_', 'mediaIndex:', mediaIndex, 'hasPlayed:', hasPlayed, 'currentTime:', currentTime, 'syncPoint:', syncPoint, 'fetchAtBuffer:', this.fetchAtBuffer_, 'bufferedTime:', bufferedTime); |
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.
Really long line
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.
And it might be a good idea to just write out checkBuffer_
src/segment-loader.js
Outdated
@@ -522,7 +515,7 @@ export default class SegmentLoader extends videojs.EventTarget { | |||
mediaIndex = mediaSourceInfo.mediaIndex; | |||
startOfSegment = mediaSourceInfo.startTime; | |||
} | |||
log('gMIFT', mediaIndex, 'sos', startOfSegment); | |||
this.logger_('gMIFT', mediaIndex, 'sos', startOfSegment); |
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.
While changing, would be good to write out getMediaIndexForTime
* | ||
* @private | ||
*/ | ||
logger_() {} |
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.
Instead of writing this function and the check for the debug setting in each module we want to log, we should just have the actual logger passed in, and check for the debug option at the highest level.
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 plan on doing this later by pulling in Pat's logger/history thing. This change was just to get rid of that ugly hack since we already have support for a less-ugly option in playback-watcher.
@mjneil Right now we are using global videojs HLS options for this. The settings object remains unchanged but the merger with the global options brings the debug flag. I decided to remove the options for sync-controller since it wasn't being used anyway. |
Pretty self-explanatory. The original logging wasn't meant to be a permanent solution.