-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Pipewire: Catch exceptions in CPipewire::Create() #23282
Conversation
} | ||
catch (const std::exception&) | ||
{ | ||
return false; |
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.
wouldn't it make sense to log the actual exception?
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 added logging, but unfortunately it doesn't show up in the log because the logger is not yet initialized. But it helps when logging directly to the console 🙂
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'm actually fine throwing if we hit undefined/unexpected behaviour. Many distros may only build Kodi with one audio sink anyways so no matter what they may have broken audio if the pipewire sink cannot initialize.
return false; | ||
try | ||
{ | ||
pipewire = PIPEWIRE::CPipewire::Create(); |
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.
We already have try/catch blocks in PIPEWIRE::CPipewire::Create
It would be nice if we could just catch the exception in there.
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.
Moved the exception handling to CPipewire::Create
.
Description
CPlatformLinux::InitStageOne()
doesn't expectOPTIONALS::*Register()
methods to throw so exceptions aren't handled, they bubble up the stack and crash the application.Motivation and context
An exception from
CAESinkPipewire::Register()
crashed Kodi in #23225 which isn't good behaviour even if the host system is setup in an expected way.How has this been tested?
Artificially create an exception in
CAESinkPipewire::Register()
. Kodi doesn't crash and instead initializes the pulseaudio interface.What is the effect on users?
Kodi doesn't crash maybe there even is sound.
Types of change
Checklist: