After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 595231 - [pulsesink] Lowers volume after every new track
[pulsesink] Lowers volume after every new track
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.15
Other Linux
: Normal critical
: 0.10.17
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 588409 595267 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-09-15 07:07 UTC by Sebastian Dröge (slomo)
Modified: 2010-04-08 12:22 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sebastian Dröge (slomo) 2009-09-15 07:07:52 UTC
Hi,
pulsesink from gst-plugins-good 0.10.16 lowers the volume after every new track in totem (but only if the volume wasn't 100%). This looks like a regression from 0.10.15...
Comment 1 Tim-Philipp Müller 2009-09-15 08:16:51 UTC
Are you sure this is a regression? I've had this kind of problem with rhythmbox / totem as in debian sid for a while now, well before the -good 0.10.16 release. Might be a playbin(2) thing as well, not sure, never looked into it.
Comment 2 Sebastian Dröge (slomo) 2009-09-15 08:27:50 UTC
Well, I noticed it not a long time ago :)
Comment 3 Jan Schmidt 2009-09-15 10:50:46 UTC
I've seen behaviour like this on newer pulseaudio with the 'global volume control' logic. Every reconnect, if the application has a volume < 100%, it would get lower each time until it (very quickly) hit 0% and sat there.
Comment 4 Marc-Andre Lureau 2009-09-15 11:40:03 UTC
you could check if it's a pulseaudio bug by modifying /etc/pulse/daemon.conf (or ~/.pulse/daemon.conf) with:

flat-volumes = no
Comment 5 Sebastian Dröge (slomo) 2009-09-15 15:43:57 UTC
*** Bug 595267 has been marked as a duplicate of this bug. ***
Comment 6 Bastien Nocera 2009-09-15 17:05:42 UTC
attachment 143227 [details] is a test case
and Bug 595267 Comment 0 has a reproducer.
Comment 7 Michael Monreal 2009-09-15 17:24:47 UTC
I have seen the same problem on Fedora 11. The "flat-volumes = no" does not really help (well: setting it will totally improve things as the global volume
control logic is totally weird), at least the decrease still happened for me. 

Setting the volume to 100% (as said in the original report) however seems to work around it.
Comment 8 René Stadler 2009-09-20 13:25:44 UTC
Can you reproduce this with gst-launch? Try:

gst-launch-0.10 -v audiotestsrc num-buffers=10 ! pulsesink volume=0.5

Then for volume restore, drop the volume=...:

gst-launch-0.10 -v audiotestsrc num-buffers=10 ! pulsesink |fgrep volume

If this works, it could be related to playbin2, try:

gst-launch-0.10 -v playbin2 uri=file:///home/.../...avi volume=0.5

Repeat again with restore:

gst-launch-0.10 -v playbin2 uri=file:///home/.../...avi |fgrep volume
Comment 9 Bastien Nocera 2009-10-05 12:11:53 UTC
$ gst-launch-0.10 -v audiotestsrc num-buffers=10 ! pulsesink volume=0.5 | fgrep volume
/GstPipeline:pipeline0/GstPulseSink:pulsesink0: volume = 0.035958

$ gst-launch-0.10 -v audiotestsrc num-buffers=10 ! pulsesink |fgrep volume
/GstPipeline:pipeline0/GstPulseSink:pulsesink0: volume = 0.071912

$ gst-launch-0.10 -v audiotestsrc num-buffers=10 ! pulsesink | fgrep volume
/GstPipeline:pipeline0/GstPulseSink:pulsesink0: volume = 0.071912

$ gst-launch -v playbin2 uri=file:///home/data/Documents/Movie\ Samples/Close_central_rounded_vowel.ogg volume=0.5 | grep volume
/GstPlayBin2:playbin20/GstPlaySink:playsink0/GstBin:abin/GstAutoAudioSink:audiosink/GstPulseSink:audiosink-actual-sink-pulse: volume = 0.035958

$ gst-launch -v playbin2 uri=file:///home/data/Documents/Movie\ Samples/Close_central_rounded_vowel.ogg | grep volume
/GstPlayBin2:playbin20/GstPlaySink:playsink0/GstBin:abin/GstAutoAudioSink:audiosink/GstPulseSink:audiosink-actual-sink-pulse: volume = 0.071912

$ gst-launch -v playbin2 uri=file:///home/data/Documents/Movie\ Samples/Close_central_rounded_vowel.ogg | grep volume
/GstPlayBin2:playbin20/GstPlaySink:playsink0/GstBin:abin/GstAutoAudioSink:audiosink/GstPulseSink:audiosink-actual-sink-pulse: volume = 0.071912
Comment 10 Lennart Poettering 2009-10-14 22:34:33 UTC
Hmm, i compiled the test case here and the volume stays the same for all streams I play. Seems to work fine. Any hint hat I can do to reproduce this?

Does this happen on all devices? Is this possibly limited to devvice without dB info?
Comment 11 Lennart Poettering 2009-10-14 22:38:59 UTC
I played around with this a little more with all six cards I have here on PA 0.9.19. The volume stays completely constant all the time. Even after 20 iterations of the program.

Can anyone reproduce this on 0.9.19?
Comment 12 Sebastian Dröge (slomo) 2009-10-15 04:00:49 UTC
I can't reproduce it anymore with 0.9.19... someone else?
Comment 13 Sebastian Dröge (slomo) 2009-10-16 11:12:28 UTC
...and happens again here.
Comment 14 Lennart Poettering 2009-10-16 12:20:42 UTC
(In reply to comment #13)
> ...and happens again here.

Any hint what I can do to reproduce this? Could you paste the "pcmd ls" output of the devcie you are experiencing this on? If you run PA with -vvvv it will print a line for each client triggered volume change. Do you see any when you reproduce this?
Comment 15 Bastien Nocera 2009-10-16 14:43:09 UTC
The problem happens when changing the volume in Totem once from the default.

1. Launch Totem
2. Start playing something
3. Change the volume in Totem
4. Skip track
5. Volume is changed from the pulsesink side

Commented debug output:
** Message: first volume set, ignoring

We ignore the saved volume when using PA.

** Message: got new PA volume 0.023758
** Message: finishing notifying about new PA volume

Notification from pulsesink about the "volume" property, we change our UI

** Message: setting PA volume 0.043758

User interaction, set the property on pulsesink.

** Message: got new PA volume 0.043758
** Message: finishing notifying about new PA volume

Notification from pulsesink about the "volume" property, we change our UI (eg. do nothing)

** Message: got new PA volume 0.043758
** Message: finishing notifying about new PA volume

Another notification.

** Message: got new PA volume 0.007861
** Message: finishing notifying about new PA volume

And after the track switch, yikes!
Comment 16 Lennart Poettering 2009-10-17 00:37:22 UTC
So here's my fix:

http://git.0pointer.de/?p=gst-plugins-good.git;a=patch;h=cee7048dcd68f1d3cca3651a5aaef11145390fa4

If you merge this, please consider merging the whole series at 

http://git.0pointer.de/?p=gst-plugins-good.git

Includes a couple of minor fixes, pretty boring, and I am too lazy to create seperate bug reports about this.
Comment 17 Lennart Poettering 2009-10-17 00:43:19 UTC
That series also includes a patch for bug 595029. So if you merge the series make sure to close that bug too.

And the git URL is this one:

git://git.0pointer.de/gst-plugins-good.git
Comment 18 Bastien Nocera 2009-10-17 01:34:48 UTC
Patch mentioned in comment 16 works for me, and fixes the problem seen in comment 15.
Comment 19 Sebastian Dröge (slomo) 2009-10-17 05:38:33 UTC
(In reply to comment #17)
> That series also includes a patch for bug 595029. So if you merge the series
> make sure to close that bug too.
> 
> And the git URL is this one:
> 
> git://git.0pointer.de/gst-plugins-good.git

Hm, shouldn't the volume setting in pa_stream_connect_playback() go away and be replaced by pa_context_set_sink_input_volume()? Because former is relative volume and latter absolute...
Comment 20 Sebastian Dröge (slomo) 2009-10-17 06:45:07 UTC
same goes for the mute state (not sure if there's a difference between setting it when connecting the stream of in the pa_context).

This should be no problem in most cases but will be a problem if the volume/mute state is set while pulsesink is in NULL state or at any other time when there's no pa_context.

I'll review the other changes and push them to master but would be nice if you could either explain that I'm wrong or could provide an additional patch :)
Comment 21 Sebastian Dröge (slomo) 2009-10-17 06:59:49 UTC
Also, shouldn't volume_set and mute_set be set to FALSE after pa_context_set_sink_input_volume()?

(I've looked at the other commits, they're all good. Thanks, I'll push them now all except the volume one)
Comment 22 Lennart Poettering 2009-10-17 12:35:49 UTC
(In reply to comment #19)
> (In reply to comment #17)
> > That series also includes a patch for bug 595029. So if you merge the series
> > make sure to close that bug too.
> > 
> > And the git URL is this one:
> > 
> > git://git.0pointer.de/gst-plugins-good.git
> 
> Hm, shouldn't the volume setting in pa_stream_connect_playback() go away and be
> replaced by pa_context_set_sink_input_volume()? Because former is relative
> volume and latter absolute...

Yes and no. Even worse than setting the volume is setting the volume asynchronously in two steps. 

The proper fix for this is that we use a new flag I just added to PA which allows clients to select whether they want to specify relative or absolute volumes. For now I'd like to leave the current logic where the volume before and after stream setup can mean something different. (which they can anyway, since before the stream setup we don't know anything about the device and the volume capabilities of the device, but afterwards we do)

I'll prep a patch for gst as soon as this new flag is stabilizied.
Comment 23 Lennart Poettering 2009-10-17 12:38:04 UTC
(In reply to comment #20)
> same goes for the mute state (not sure if there's a difference between setting
> it when connecting the stream of in the pa_context).
> 
> This should be no problem in most cases but will be a problem if the
> volume/mute state is set while pulsesink is in NULL state or at any other time
> when there's no pa_context.
> 
> I'll review the other changes and push them to master but would be nice if you
> could either explain that I'm wrong or could provide an additional patch :)

I don't see how the mute state could ever mean something different before and after stream setup. It's a boolean. Which thankfully does not leave much room for vague definitions. ;-)
Comment 24 Lennart Poettering 2009-10-17 12:41:27 UTC
(In reply to comment #21)
> Also, shouldn't volume_set and mute_set be set to FALSE after
> pa_context_set_sink_input_volume()?

Not necessary. We only ever set those flags *before* the stream is setup -- where we reset them again. While the stream is established the flags are not used at all anymore, are hence neither set, nor reset.
Comment 25 Bastien Nocera 2009-10-17 13:35:16 UTC
I tested the tip of Lennart's branch, and after a few track switches, even though the volume doesn't change, data stops being sent to pulseaudio.

The stream still appears as a client, but the monitor bar in pavucontrol is empty. I'll try to bisect...
Comment 26 Sebastian Dröge (slomo) 2009-10-17 15:07:21 UTC
(In reply to comment #24)
> (In reply to comment #21)
> > Also, shouldn't volume_set and mute_set be set to FALSE after
> > pa_context_set_sink_input_volume()?
> 
> Not necessary. We only ever set those flags *before* the stream is setup --
> where we reset them again. While the stream is established the flags are not
> used at all anymore, are hence neither set, nor reset.

Not really, they're also set in gst_pulsesink_set_volume(), which is called whenever the "volume" property is set (and the same for mute of course).

(In reply to comment #22)
> (In reply to comment #19)
> > Hm, shouldn't the volume setting in pa_stream_connect_playback() go away and be
> > replaced by pa_context_set_sink_input_volume()? Because former is relative
> > volume and latter absolute...
> 
> Yes and no. Even worse than setting the volume is setting the volume
> asynchronously in two steps. 
> 
> The proper fix for this is that we use a new flag I just added to PA which
> allows clients to select whether they want to specify relative or absolute
> volumes. For now I'd like to leave the current logic where the volume before
> and after stream setup can mean something different. (which they can anyway,
> since before the stream setup we don't know anything about the device and the
> volume capabilities of the device, but afterwards we do)
> 
> I'll prep a patch for gst as soon as this new flag is stabilizied.

Wouldn't it be better then to *not* set the mute/volumes in stream setup and only afterwards? Otherwise the volume property has two different meanings, depending on the current state.... and that's not really nice.
Comment 27 Lennart Poettering 2009-10-17 18:59:51 UTC
(In reply to comment #26)
> (In reply to comment #24)
> > (In reply to comment #21)
> > > Also, shouldn't volume_set and mute_set be set to FALSE after
> > > pa_context_set_sink_input_volume()?
> > 
> > Not necessary. We only ever set those flags *before* the stream is setup --
> > where we reset them again. While the stream is established the flags are not
> > used at all anymore, are hence neither set, nor reset.
> 
> Not really, they're also set in gst_pulsesink_set_volume(), which is called
> whenever the "volume" property is set (and the same for mute of course).

No. Look more closely! My patch made sure that they are only set when the stream is not set up yet. If it is set up we just call pa_sink_input_set_volume() and do not store the volume locally.

> > I'll prep a patch for gst as soon as this new flag is stabilizied.
> 
> Wouldn't it be better then to *not* set the mute/volumes in stream setup and
> only afterwards? Otherwise the volume property has two different meanings,
> depending on the current state.... and that's not really nice.

It only has a different meaning for some sinks, not for all. That said, this is indeed ugly, which is why I have added a flag to the PA API that allows clients to say if they want relative or absolute volumes. However without this flag you simply cannot say what you get, neither during stream creation nor while the stream exists. This is a weakness of the API. Will be fixed. 

I am contemplating making an additional change in PA itself: by default make volumes configured by clients absolute. That would eliminate the uncertainty too. And I wouldn't even consider that breakage since before that we did no specify the type of volumes for streams at all.
Comment 28 Bastien Nocera 2009-10-17 23:20:44 UTC
(In reply to comment #25)
> I tested the tip of Lennart's branch, and after a few track switches, even
> though the volume doesn't change, data stops being sent to pulseaudio.
> 
> The stream still appears as a client, but the monitor bar in pavucontrol is
> empty. I'll try to bisect...

Couldn't reproduce it, so I'll have to admit it might be a problem with Totem 2.26.x and playbin1.
Comment 29 Sebastian Dröge (slomo) 2009-10-18 08:17:33 UTC
(In reply to comment #27)
> (In reply to comment #26)
> > (In reply to comment #24)
> > > (In reply to comment #21)
> > > > Also, shouldn't volume_set and mute_set be set to FALSE after
> > > > pa_context_set_sink_input_volume()?
> > > 
> > > Not necessary. We only ever set those flags *before* the stream is setup --
> > > where we reset them again. While the stream is established the flags are not
> > > used at all anymore, are hence neither set, nor reset.
> > 
> > Not really, they're also set in gst_pulsesink_set_volume(), which is called
> > whenever the "volume" property is set (and the same for mute of course).
> 
> No. Look more closely! My patch made sure that they are only set when the
> stream is not set up yet. If it is set up we just call
> pa_sink_input_set_volume() and do not store the volume locally.

Doh, sorry. Pushed now, thanks for the patch :)

commit e4d6a2aa2c6b8b3d9137a38e10d28434eb31a580
Author: Lennart Poettering <lennart@poettering.net>
Date:   Sat Oct 17 02:18:53 2009 +0200

    pulse: never apply volume more than once
    
    Generally decisions on the volume of the stream should be done inside of
    PA, not inside of Gst. Only PA knows how volumes translate between
    devices and s on.
    
    This patch makes sure that all volumes set via the volume property are
    only applied *once* to the underlying stream. After applying them the
    client side will not store them anymore. This should make sure that
    really only user-triggered volume changes are forwarded to server, but
    the client never tries to save/restore the volume internally.
    
    Fixes bug #595231.

> I am contemplating making an additional change in PA itself: by default make
> volumes configured by clients absolute. That would eliminate the uncertainty
> too. And I wouldn't even consider that breakage since before that we did no
> specify the type of volumes for streams at all.

Yes, make absolute volume the default and I'll be completely happy ;)
Comment 30 Sebastian Dröge (slomo) 2009-10-18 08:18:46 UTC
(In reply to comment #28)
> (In reply to comment #25)
> > I tested the tip of Lennart's branch, and after a few track switches, even
> > though the volume doesn't change, data stops being sent to pulseaudio.
> > 
> > The stream still appears as a client, but the monitor bar in pavucontrol is
> > empty. I'll try to bisect...
> 
> Couldn't reproduce it, so I'll have to admit it might be a problem with Totem
> 2.26.x and playbin1.

I have this with 2.28/playbin2 too sometimes (but had this before Lennart's patches too). Unfortunately I found no reliable way to reproduce it yet. Would you mind filing a new bug for this? :)
Comment 31 Colin Guthrie 2009-10-18 11:53:57 UTC
I'm also seeing this "sticking" issue (both with 0.10.16 and and git tip of the pulse stuff via cherry picking), but for me, every track change, or position seek is causing things to lock up. It seems to free itself sometimes after some amount of time... not sure what is causing it. If there is another bug reference for this please post it as reference :)
Comment 32 Colin Guthrie 2009-10-18 12:14:24 UTC
Actually scratch that last comment, I was being a doofus. I do seem to be having a problem with 42ee5e22a2e696e2d8390e89f90d78b149417944 tho', despite having 0.10.25 for base stuff (which contains the baseaudiosink changes). If I include this patch in my cherry picking the track changing stuff borks royally. Anyway this is offtopic for this bug. I'll try and investigate further but my priority just now is getting things stable for the Mdv release. Wim, feel free to ping me on IRC about this if you like.
Comment 33 Tobias Mueller 2010-04-08 12:22:46 UTC
*** Bug 588409 has been marked as a duplicate of this bug. ***