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.

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: #3558 #3559

Merged
merged 1 commit into from
Mar 29, 2021
Merged

fix: #3558 #3559

merged 1 commit into from
Mar 29, 2021

Conversation

orange4glace
Copy link
Contributor

@orange4glace orange4glace commented Feb 27, 2021

fix #3558

Set buffering time to last quota exceeded segment time on QuotaExceededError occurs

@dsilhavy dsilhavy added this to the 3.3.0 milestone Mar 2, 2021
@dsilhavy dsilhavy self-requested a review March 2, 2021 12:29
@dsilhavy
Copy link
Collaborator

dsilhavy commented Mar 4, 2021

@orange4glace Thanks for this PR.

Following our contribution guidelines, and since this seems to be your first PR, could you please send me a signed copy of dash.js feedback agreement?

@orange4glace
Copy link
Contributor Author

Thanks for the response, @dsilhavy !
I've uploaded my agreement here. https://groups.google.com/g/dashjs/c/3613JLAYdG4

@dsilhavy
Copy link
Collaborator

@orange4glace In your tests did you verify that this does not interfere with the logic that is applied after the buffer has been cleared? In StreamProcessor.onBufferCleared:

 // If buffer removed ahead current time (QuotaExceededError or automatic buffer pruning) then adjust current index handler time

 if (e.from > playbackController.getTime()) {
            bufferingTime = e.from;
            bufferPruned = true;
 }

@orange4glace
Copy link
Contributor Author

orange4glace commented Mar 26, 2021

Currently, clearBuffers(getClearRanges()) fires after QUOTA_EXCEEDED is triggered.
Even though, since getClearRanges() returns ranges only before the currentTime, it seems that there's no chance that if (e.from > playbackController.getTime()) and QUOTA_EXCEEDED occurs simultaneously at least in the current codebase.

            if (e.error.code === QUOTA_EXCEEDED_ERROR_CODE || !hasEnoughSpaceToAppend()) {
                logger.warn('Clearing playback buffer to overcome quota exceed situation');
                // Notify Schedulecontroller to stop scheduling until buffer has been pruned
                triggerEvent(Events.QUOTA_EXCEEDED, {
                    criticalBufferLevel: criticalBufferLevel,
                    quotaExceededTime: e.chunk.start
                });
                clearBuffers(getClearRanges());
            }

@dsilhavy dsilhavy merged commit 8af472f into Dash-Industry-Forum:development Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dash.js ignores a segment if QuotaExceedError occurs
2 participants