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

fix(FEC-11136): Tizen 4 got stuck on the beginning of the media #140

Merged
merged 14 commits into from
Apr 27, 2021
Merged

fix(FEC-11136): Tizen 4 got stuck on the beginning of the media #140

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ddf90ed
fix(FEC-11136): config to break possible stall on the beginning in Sm…
Yuvalke Apr 25, 2021
8a92e05
Update dash-adapter.js
Yuvalke Apr 25, 2021
4be1365
Update dash-adapter.js
Yuvalke Apr 25, 2021
a0d515e
Update dash-adapter.js
Yuvalke Apr 25, 2021
1cd14a6
keep the last current time changed
Yuvalke Apr 26, 2021
baa2ec3
fix for pause
Yuvalke Apr 26, 2021
81c227f
fix stall after startTime
Yuvalke Apr 26, 2021
ffc26fb
add max attempt of stall
Yuvalke Apr 26, 2021
68a6b42
Update dash-adapter.js
Yuvalke Apr 27, 2021
451d552
Merge branch 'master' into FEC-11136
Yuvalke Apr 27, 2021
6da4acb
Update dash-adapter.js
Yuvalke Apr 27, 2021
e63f396
Update dash-adapter.js
Yuvalke Apr 27, 2021
61e5387
Update dash-adapter.js
Yuvalke Apr 27, 2021
36d18fd
Update dash-adapter.js
Yuvalke Apr 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/dash-adapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ export default class DashAdapter extends BaseMediaSourceAdapter {
*/
_videoSizeUpdateTimer: ?IntervalID = null;

/**
* stall interval to break the stall on Smart TV
* @type {null|number}
* @private
*/
_stallInterval: ?IntervalID = null;

/**
* 3016 is the number of the video error at shaka, we already listens to it in the html5 class
* @member {number} - VIDEO_ERROR_CODE
Expand Down Expand Up @@ -365,10 +372,42 @@ export default class DashAdapter extends BaseMediaSourceAdapter {
}
this._maybeSetFilters();
this._maybeSetDrmConfig();
this._maybeBreakStalls();
this._shaka.configure(this._config.shakaConfig);
this._addBindings();
}

_clearStallInterval(): void {
if (this._stallInterval) {
clearInterval(this._stallInterval);
this._stallInterval = null;
}
}

_stallSmartTVHandler(): void {
this._clearStallInterval();
let lastCurrentTime = this._videoElement.currentTime;
this._stallInterval = setInterval(() => {
if (lastCurrentTime === this._videoElement.currentTime) {
this._videoElement.currentTime = parseFloat(this._videoElement.currentTime.toFixed(1)) + 0.1;
Copy link
Contributor

Choose a reason for hiding this comment

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

move 0.1 to const, maybe expose as config?

} else {
this._clearStallInterval();
}
}, 2 * 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

move to const, maybe expose as config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't want to do it too robust cause I believe we need to handle it on Shaka, WDYT?

Copy link
Contributor Author

@Yuvalke Yuvalke Apr 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

To create 3 configs for hack we're adding for edge case that we won't need anymore after Shaka will handle it, it sounds too robust, don't you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

agree, let's start lean, but please move to consts and we will hope Shaka will approve this :-)

}

/**
* register to event to break the stalls on smart TV
* @returns {void}
* @private
*/
_maybeBreakStalls(): void {
if (this._config.playback.forceBreakStall) {
this._eventManager.listenOnce(EventType.STALLED, this._stallSmartTVHandler);
this._eventManager.listenOnce(EventType.SEEKED, this._stallSmartTVHandler);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need/want to remove them after one handler is called?
If stalled occurred we can remove seeked and vide versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I clear the stall interval if it's not stopped for each one of them.
Yes, the issue that we saw occur only on the beginning.

}
}

/**
* get the redirected URL
* @param {string} url - The url to check for redirection
Expand Down Expand Up @@ -729,6 +768,7 @@ export default class DashAdapter extends BaseMediaSourceAdapter {
this._responseFilterError = false;
this._manifestParser = null;
this._thumbnailController = null;
this._clearStallInterval();
this._clearVideoUpdateTimer();
if (this._eventManager) {
this._eventManager.removeAll();
Expand Down