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

EndTime: fix overflow when doing large comparisons #22471

Merged
merged 9 commits into from Jan 15, 2023

Conversation

fritsch
Copy link
Member

@fritsch fritsch commented Jan 15, 2023

Backport of: #22452 and #22416 as they both logically belong together.

  • Fixes Stream-Silence for Audio
  • Fixes various overflows when using new chrono infrastructure, which is already in Nexus

@fritsch fritsch requested review from lrusak and thexai January 15, 2023 09:41
@fritsch fritsch added v20 Nexus Type: Backport Type: Fix non-breaking change which fixes an issue labels Jan 15, 2023
@fritsch fritsch added this to the Nexus 20.1 milestone Jan 15, 2023
@thexai
Copy link
Member

thexai commented Jan 15, 2023

Is need also backport of #22416 to avoid int overflow here:

m_settings.silenceTimeout = settings->GetInt(CSettings::SETTING_AUDIOOUTPUT_STREAMSILENCE) * 60000;

The struct member rename isn't needed but it makes it more clear that the value is in minutes from an int

Signed-off-by: Lukas Rusak <lorusak@gmail.com>
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
… casted to templated type

This is required because we use the default std::chrono::steady_clock::duration
which typically is std::chrono::nanoseconds. When comparing durations the templated type
may overflow when comparing to std::chrono::nanoseconds.

std::chrono::nanoseconds::max() on my platform works out to ~292 years

Signed-off-by: Lukas Rusak <lorusak@gmail.com>
…eaming

Signed-off-by: Lukas Rusak <lorusak@gmail.com>
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
…s silence timeout

This is required because the setting itself is stored as an int. The int represents minutes and when converted
to std::chrono::minutes will overflow std::chrono::nanoseconds which is the units of the timer.
@fritsch
Copy link
Member Author

fritsch commented Jan 15, 2023

@thexai Thanks much, that one slipped through.

Copy link
Contributor

@lrusak lrusak left a comment

Choose a reason for hiding this comment

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

Looks like everything was included to properly fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Backport Type: Fix non-breaking change which fixes an issue v20 Nexus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants