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

Send overall (rather than per-stage) completion ratio to Studio #33

Merged
merged 4 commits into from
Sep 2, 2022

Conversation

thomshutt
Copy link
Contributor

No description provided.

Copy link
Member

@victorges victorges left a comment

Choose a reason for hiding this comment

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

Cool!


// Define the "base" numbers - e.g the overall ratio we start each stage at
var TranscodeStatusPreparingBase float64 = 0.0
var TranscodeStatusPreparingTranscodingBase float64 = 0.4
Copy link
Member

Choose a reason for hiding this comment

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

Should it be only "Transcoding"?

Suggested change
var TranscodeStatusPreparingTranscodingBase float64 = 0.4
var TranscodeStatusTranscodingBase float64 = 0.4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "Base" was trying to signify that this is the lower limit / floor / whatever, open to suggestions for better names though. Maybe "Floor"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just realised what you were actually pointing out 👓

case TranscodeStatusPreparingCompleted:
return TranscodeStatusPreparingTranscodingBase
case TranscodeStatusTranscoding:
return TranscodeStatusPreparingTranscodingBase + (currentStageCompletionRatio * (TranscodeStatusCompletedBase - TranscodeStatusPreparingTranscodingBase))
Copy link
Member

Choose a reason for hiding this comment

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

Another way of implementing this, to have the more complex math only in 1 place, would be to have 2 vars "start" and "end" and force the currentStageCompletionRatio to be 1 on the "Completed" steps (but use their same start/end as the non-completed). Sth like

	switch status {
	case TranscodeStatusPreparing, TranscodeStatusPreparingCompleted:
		start, end = 0, 0.4
	case TranscodeStatusTranscoding, TranscodeStatusTranscodingCompleted:
		start, end = 0.4, 1
        }
        if (status == TranscodeStatusPreparingCompleted || status == TranscodeStatusTranscodingCompleted) {
                currentStageCompletionRatio = 1
        }

Then there could be a single progress-scaling function like this: https://github.com/livepeer/task-runner/blob/4d1ae1768c01e979969bf7f00b81267db05e299c/task/progress.go#L66

Certainly not a required change, just tripping on the impl. Could make sense when we start adding more steps here. WDYT?

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 like that a lot!

Comment on lines +110 to +115
switch status {
case TranscodeStatusPreparing, TranscodeStatusPreparingCompleted:
return scaleProgress(currentStageCompletionRatio, 0, 0.4)
case TranscodeStatusTranscoding, TranscodeStatusCompleted:
return scaleProgress(currentStageCompletionRatio, 0.4, 1)
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@thomshutt thomshutt merged commit 1753b3b into main Sep 2, 2022
@thomshutt thomshutt deleted the overall-completion-ratiomake branch September 2, 2022 16:58
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.

2 participants