Need to set VLC audio device after setting media source
Needs RevisionPublic

Authored by marten on May 4 2019, 12:34 PM.

Details

Reviewers
sitter
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Bug https://bugs.kde.org/show_bug.cgi?id=400979 describes a problem in the Phonon KCM (System Settings - Hardware - Audio and Video - Device Preference) where the "Test" button, contrary to its description, always plays the test sound through the default sound device. This can also be verified with a command line audio playing tool such as http://github.com/martenjj/phononplay, where even if an explicit output device is specified the sound always plays through the default device.

According to comments at https://forum.videolan.org/viewtopic.php?t=98766, the VLC output device must be set (using libvlc_audio_output_device_set) after setting the media source (using libvlc_media_player_set_media). However, in the current implementation of Phonon it is set (in MediaPlayer::setAudioOutputDevice) before setting the media source (in MediaPlayer::setMedia which is called by MediaObject::play).

Because libvlc_media_player_set_media is called internally by MediaObject::play, it is not possible to do these calls in the correct order from either the KCM or any other Phonon application.

This patch fixes the order of the libvlc calls. It is trying to be as simple as possible and so simply saves the device parameters to MediaPlayer::setAudioOutputDevice as well as setting the device there and then (for compatibility). If those parameters have been saved then it sets them again in MediaPlayer::setMedia after the media source has been set. No changes to the KCM, any calling applications or the Phonon API are required.

With this change, the KCM works correctly and sound can be played through any specified device.

Test Plan

Verified that without this change the Phonon KCM always plays the test sound through the default audio device, and specifying a device for the command line player is ignored.

Built phonon-vlc with this change. Verified that the Phonon KCM plays the test sound through the audio device that is selected in the list, and the command line player uses the specified device. Checked that all Phonon applications play sound as expected.

Diff Detail

Repository
R489 Phonon: VLC backend
Lint
Lint Skipped
Unit
Unit Tests Skipped
marten requested review of this revision.May 4 2019, 12:34 PM
marten created this revision.
sitter requested changes to this revision.May 6 2019, 9:28 AM
sitter added a subscriber: sitter.

I think we'll want upstream's take on this. In the forum post you reference Remi does say that he suspects a bug at play, and I concur with him. When a property is set on the player it ought to persist unless there is an obvious reason why it wouldn't. In the case of the output device I do not see why the player should forget the device I told it to use just because the media instance changes.

Furthermore your code actually ought to go into the audio outputs. They are SinkNodes and that class has addToMedia which is specifically to deal with doing things once the media instance was created.

Also, I am guessing this doesn't actually fix it on pulseaudio, where I think there is further breakage caused by the fact that the a) the pulse audio output in libvlc has no support for setting the device and b) as a result us not setting any meaningful device name.

This revision now requires changes to proceed.May 6 2019, 9:28 AM
sitter added a subscriber: jbkempf.May 6 2019, 9:28 AM

@jbkempf may have input on the libvlc side?