Quoting Max Kellermann <[email protected]>:
Your code will crash when no "sink" is configured, because the
std::string is initialized with a nullptr.
[sigh]. verified, yes
What happens if the sink that your code remembers suddenly vanishes
while MPD is not playing, will reopening the output always fail until
somebody creates a new sink with that name?
[multiple sighs]. verified, you're absolutely right yes, however, the
same issue would occur if a sink set in the config was removed, hence
it's an existing issue
i've forced through fixes to these issues in the original branch,
'pulse.dyn.sink' but..
the first fix (for 'segfault if no sink set') made me revert my commit
completely and test behaviour with no explicit sink set (a
configuration i've not used in, well, for ever). and what do you know,
letting the (pa) server decide how a 'previously seen application's
stream is handled works perfectly, just as i'd originally
wanted/expected it to
hence the patch scope/reasoning needs changing to 'when sink is
explicitly set via config do..'
the second fix (for 'no connection if sink is invalidated'), just
ensured a failing connection reverted to the default - using a
nullptr' aka letting the (pa) server make the decisions
so there's a theme here right! trusting the (pa) server comes highly
recommended in the api, and coupled with your..
I'm not convinced that this is the best idea.. ..Isn't there some kind of
server-side stickiness?
..i thought i'd best have a rethink! so,
Does every Pulse client have to replicate this behavior?
i don't know. most i use give the option to specify a sink, but if
they don't implement something to deal with the issue here then i'd
imagine most can get away with it as the number connections/new
streams per session is usually ~1, not 100s ..we'd have to look at
other audio players specifically
Isn't there a better way? Can we change an existing stream to a
different audio format instead, instead of destroying and creating
a new one?
i don't think that needs to be addressed for this patch. the issue of
resetting obviously also occurred for stopping and starting of the
player. so the idea of storing the current sink in the (persistent
over streams) audio output structure was good (imo only) as it worked
for both scenarios
but if that won't fly, then given the above theme of 'if in doubt
trust the server', how about just invalidating any sink (set by
config) if we detect it having moved, and let pulse do its thing?
so please see branch 'pulse.dyn.sink.v2'
pre-empting complaints :D
-casting away the const flag on the po->sink pointer to allow runtime
setting to nullptr. this causes a compiler warning, and is usually
frowned upon. but the use is noted in the code, and we're not
manipulating the underlying so, dare i say 'why not?' failing that,
one could use a bool variable, set true when the stream is moved, and
then dependent on this, use/ignore custom sink name
-catching the pulse 'entity not found' error, and re-initialising the
connection. no error checking, for audio format creation, or in the
re-initialisation. my assumption in doing this is that the connection
to the pa server has already succeeded earlier in the code
cheers,
Pete
_______________________________________________
mpd-devel mailing list
[email protected]
http://mailman.blarg.de/listinfo/mpd-devel
