Bug 321172 - setVolume before play() on PA systems will be overwritten/ignored by PA
Summary: setVolume before play() on PA systems will be overwritten/ignored by PA
Status: RESOLVED FIXED
Alias: None
Product: Phonon
Classification: Frameworks and Libraries
Component: Pulsesupport (show other bugs)
Version: 4.6.0
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: 4.6.1
Assignee: Harald Sitter
URL:
Keywords:
: 255246 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-06-15 11:53 UTC by kripton
Modified: 2013-10-09 13:59 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.7.0


Attachments
phonon-setVolume.log (21.89 KB, text/plain)
2013-06-15 14:22 UTC, kripton
Details
phonon-workaround.patch (913 bytes, patch)
2013-06-15 14:26 UTC, kripton
Details
phonon-setVolume-workaround.log (21.56 KB, text/plain)
2013-06-15 14:27 UTC, kripton
Details
phonon-workaround.patch (1.42 KB, patch)
2013-06-15 14:29 UTC, kripton
Details
volumetest_wovids.tar.xz (1.90 KB, application/x-xz)
2013-06-15 20:24 UTC, kripton
Details
phonon-two-videoplayer.log (20.05 KB, text/plain)
2013-06-15 20:26 UTC, kripton
Details

Note You need to log in before you can comment on or make changes to this bug.
Description kripton 2013-06-15 11:53:08 UTC
I am using Phonon on a system using pulseaudio-4.0 and pulse's JACK backend. In my Qt4-application, I have one Phonon::VideoPlayer. When I try to set the output volume via "ui->VideoPlayer->setVolume(0.0);", nothing happens.
I changed phonon's "AudioOutput::setVolume(qreal volume)"-function to not check for pulse at all and use the "INTERFACE_CALL" code path unconditionally which fixes the problem for me.

Trying Gentoo's "live" package of Phonon (= current git HEAD, branch "master"), the behaviour is the same.

Might be related to https://bugs.meego.com/show_bug.cgi?id=14318 (which is old but still "NEW" and no hint of ever getting in contact with upstream (=Phonon). The source code posted there (tar archive) can be used as a quick example showing how to reproduce the bug.

Reproducible: Always

Steps to Reproduce:
see https://bugs.meego.com/show_bug.cgi?id=14318
Actual Results:  
see https://bugs.meego.com/show_bug.cgi?id=14318

Expected Results:  
see https://bugs.meego.com/show_bug.cgi?id=14318
Comment 1 Harald Sitter 2013-06-15 14:01:24 UTC
Please get a debug log. http://techbase.kde.org/Development/Tutorials/Debugging/Phonon#Environment_Variables
Comment 2 kripton 2013-06-15 14:22:41 UTC
Created attachment 80530 [details]
phonon-setVolume.log

terminal-log with debugging enabled. tar archive from https://bugs.meego.com/show_bug.cgi?id=14318 using three volume levels (0.00, 0.50 and 1.00). Everytime, the sample is played with default volume (guess: 1.00)
Comment 3 kripton 2013-06-15 14:26:42 UTC
Created attachment 80531 [details]
phonon-workaround.patch

Patch I use a workaround. Not a fix.
Comment 4 kripton 2013-06-15 14:27:23 UTC
Created attachment 80532 [details]
phonon-setVolume-workaround.log

terminal log when running the application with phonon with workaround patch applied
Comment 5 kripton 2013-06-15 14:29:45 UTC
Created attachment 80533 [details]
phonon-workaround.patch

re-upload patch. obsolete one was formatted badly
Comment 6 Harald Sitter 2013-06-15 14:52:24 UTC
"PulseSupport(2): Phonon Output Stream {a240f415-7ddd-4cd6-ac67-a8f86ffb1031} is gone at the PA end. Marking it as invalid in our cache as we may reuse it." 
[...]
"PulseSupport(2): Attempting to set volume to 0 for Output Stream {a240f415-7ddd-4cd6-ac67-a8f86ffb1031}" 

I reckon that's bound to fail.

I'll argue that the code is wrong though...
    mediaObject->setCurrentSource(Phonon::MediaSource(ui->filename->text()));
    audioOutput->setVolume(ui->doubleSpinBox->value());
    Phonon::Path path = Phonon::createPath(mediaObject, audioOutput);
    mediaObject->play();
^ that is assuming that phonon will actually apply the value once playing starts, which is undefined behavior at best. similarly to how you cannot seek on a not playing|paused|buffering mediaobject.

the problem of course is that PulseSupport does not cache values. if one calls setVolume with PA it will try to set the volume, fail to find the related stream and give up [1]; a system without PA will set the value in the AO frontend class which forwards it to the backend once the backendObject is created (the backend object then again caches it until the pipeline is constructed...).

the right way to do this would be
1. create a slot to handle stateChanged to Playing|Buffering|Paused to call setVolume and connect the MO to it
3. createPath
3. setCurrentSource
4. play
[5. when changing state to a playesque state the volume is set through the stateChanged slot]

yet we do similar caching in other parts of Phonon and in this particular case it actually improves easy of use of the API so PASupport probably needs to introduce caching one way or the other.

marking confirmed.

[1] https://projects.kde.org/projects/kdesupport/phonon/phonon/repository/revisions/d0f535573680468d0014b326238bbc18f735a302/entry/phonon/pulsesupport.cpp#L1241
Comment 7 kripton 2013-06-15 14:59:20 UTC
I just used that example since it was already there and reasonably small. In the code of the project I'm working on, I try to setVolume() when the state is "playing", not before. And it fails there as well. Do I need to pause first before I do the call to setVolume()? Reading through the code it doesn't look like.
When my application is playing the video, I can use any pulseaudio-mixer application (kmix in my case) to modify the volume of the Qt application's audio stream to pulseaudio.
Thanks for looking into this!
Comment 8 Harald Sitter 2013-06-15 15:09:29 UTC
Sounds like a different bug then.

Supposedly PASupport also doesn't have the right stream when you call setVolume but due to a different reason, in the example shown it most definitely is because there is no stream because nothing is playing yet. Perhaps you could create an example that matches what your application is doing?
Comment 9 kripton 2013-06-15 17:50:51 UTC
It's starting to behave odd. In a minimal example I derived from the volumetest example from meego's bugzilla, Phonon behaves as expected:
"PulseSupport(2): Attempting to set volume to 0 for Output Stream {92128bc6-8d9e-43da-8020-89212329d719}" 
"PulseSupport(2): Found PA index 164. Calling pa_context_set_sink_input_volume()" 
"PulseSupport(2): Found PulseAudio stream index 164 for Phonon Output Stream {92128bc6-8d9e-43da-8020-89212329d719}"

In my real application, things don't work so well:
"PulseSupport(2): Attempting to set volume to 0 for Output Stream {383d4a4a-fcc0-407d-b160-55f629d53cae}" 
MUTED. Volume now: 0

I'll keep investigating an post updates here as soon as I found the cause
Comment 10 kripton 2013-06-15 20:24:39 UTC
Created attachment 80539 [details]
volumetest_wovids.tar.xz

I found out what my problem is. I use more than one Phonon::VideoPlayer objects (two in the example projects). The setVolume() call works on the VideoPlayer started last but not on the other ones.

Archive with test project. Needs two video files "test1.ogg" and "test2.ogg". Archive with example videos is here:
http://kripton.kripserver.net/n0th1ng/volumetest.tar.xz
Comment 11 kripton 2013-06-15 20:26:25 UTC
Created attachment 80540 [details]
phonon-two-videoplayer.log

Log from command line when running the attached example project (attachment 80539 [details])
Comment 13 Harald Sitter 2013-06-17 00:37:15 UTC
The new example code presents the very same issue and still does not mute in Playing|Paused|Buffering

void VideoBox::play(Phonon::MediaSource *source)
{
    ui->videoPlayer->play(*source);
    mute();
Comment 14 Harald Sitter 2013-06-17 11:15:24 UTC
found the problem it's non-trivial though. also, please file a new bug report the originally presented example highlights a different bug.

<apachelogger> qputenv("PULSE_PROP_phonon.streamid", streamUuid.toLatin1());
<apachelogger> what happens is that we set the env *when creating the frontend AO*, wheras it is forwarded to PA *when the gst/vlc create their stream*
<apachelogger> so we have AO1::AO1() -> uuid foo; AO2::AO2() -> uuid bar;
<apachelogger> vlc::createAO1(); vlc::createAO2();
<apachelogger> in both cases the envrionment says uuid bar
<apachelogger> what with AO2 now having overwritten AO1

so the first AO will, from a PASupport POV, identify itself with the same UUID as the second AO effectively making whatever pulseaudio stream gets added last become the one and only pulse stream we know about. so, two AOs are broken because we fail to map them to their respective pulse streams correctly.
Comment 15 Harald Sitter 2013-07-04 00:03:20 UTC
*** Bug 255246 has been marked as a duplicate of this bug. ***
Comment 16 Harald Sitter 2013-07-04 00:52:52 UTC
Git commit 23795b6e3feb6aeea50335566174193378beb589 by Harald Sitter.
Committed on 04/07/2013 at 00:52.
Pushed by sitter into branch 'master'.

cache volume inside PAStreams when trying to set it on an invalid one

non-PA phonon caches all and everything and also the volume. in particular
you can set the volume before reaching playing state (which in fact is
not supported by VLC nor PulseSupport). Yet from a convenience POV this
has actual use so this change introduces caching inside PAStream.

When PASupport gets a cb about sink changes it tries to update the volume
in our PAStream accordingly. Before doing so we now try to apply a
possibly cached volume using queued invoke method as right now it's not
clear whether it's safe to change the volume from inside the callback
thread.

I'd like to get this fix verified by at least some people using knotify4
before considering the issue actually resolved. WRT knotify simply trying
to change the volume setting inside the KCM will do (upon sound output
the notification volume in pavucontrol/kmix should update accordingly)

M  +25   -2    phonon/pulsestream.cpp
M  +10   -2    phonon/pulsestream_p.h
M  +7    -3    phonon/pulsesupport.cpp

http://commits.kde.org/phonon/23795b6e3feb6aeea50335566174193378beb589
Comment 17 Harald Sitter 2013-07-24 21:05:11 UTC
Considered fixed in git master of phonon. Testing would be much appreciated.
Comment 18 shao lo 2013-10-09 13:46:20 UTC
I'm an Ubuntu user (Kubuntu specifically).  Has this patch been pushed out?  What package will it be in?  I'm still seeing this or a similar problem, so before reporting a new bug I'd like to know if I actually have the fix on my (raring) system.
Comment 19 Harald Sitter 2013-10-09 13:49:59 UTC
(In reply to comment #18)
> if I actually have the fix on my
> (raring) system.

You don't Phonon 4.7 is not released yet.
Comment 20 shao lo 2013-10-09 13:59:36 UTC
dpkg - s phonon reports
Version: 4:4.7.0really4.6.0-0ubuntu2
I'm assuming that means I've got some something between 4.6 and 4.7?
when is phonon 4.7 going out?
Is there any way this fix could be backported if it is not soon?