Merge lp://staging/~hypodermia/ubuntu/oneiric/compiz/fix-for-bug-301174 into lp://staging/ubuntu/oneiric/compiz

Thanks for this patch and help to make ubuntu better.

The review should be done by smspillaz, Sam can you have a look please?

Approve +1

We discussed having this a long time ago but never actually got around to implementing it. Thanks.

Oh, one thing, could you retarget this against lp:compiz-core ?

I'm not a compiz dev, so my input may be overruled by someone who is, but I think this should use a named event sound rather than manually specifying a filename - that way, (a) depending on sound-theme-freedesktop (which libcanberra does) guarantees that the sound file exists, and (b) allows the sound-theme to be configured in the “Sound” capplet, with the rest of the sounds.

http://0pointer.de/public/sound-naming-spec.html suggests “bell-window-system” (which should fallback to “bell” in the freedesktop theme) would be the right choice.

This looks like it should go upstream first - Sam, do you have any comments on this?

Discussion with Sam in #ayatana:

16:07 <RAOF> It should use an event sound from http://0pointer.de/public/sound-naming-spec.html rather than manually specifying a filename. That way, sound themes will actually work.
16:07 <RAOF> Also, it becomes simpler. Bonus!
16:07 <RAOF> (I'd suggest bell-window-system)
16:10 <RAOF> Ah. Alternatively, that spec could be a pack of lies, and there not actually _be_ a bell-window-system. Superlative. There is, however, a bell. So the not-manually-specifying-a-filename thing stands.
16:17 <RAOF> Ah, again no. bell-window-system should alledgedly fall back to just “bell”
16:18 <RAOF> smspillaz: Anyway, the summary is: don't specify a filename, so that it respects the system sound theme :)
16:19 <smspillaz> RAOF: ah, ok
16:19 <RAOF> Note: this advice may need actual, you know, _testing_
16:19 <smspillaz> that doesn't exist in compizland
16:19 <RAOF> But if everything works as advertised, that's what should happen!
16:21 <smspillaz> RAOF: so we need to specify bell-window-system
16:21 <smspillaz> and instead of CA_PROP_MEDIA_FILENAME ...
16:21 <RAOF> Right.
16:22 <RAOF> And instead of CA_PROP_MEDIA_FILENAME, you *don't* specify CA_PROP_MEDIA_FILENAME, and libcanberra looks it up in the sound theme.
16:22 <smspillaz> I hope canberra uses gtkdoc ...
16:22 <RAOF> Why, yes it does.
16:22 <RAOF> http://0pointer.de/lennart/projects/libcanberra/gtkdoc/libcanberra-canberra.html#CA-PROP-EVENT-ID:CAPS
16:22 <smspillaz> oh I see
16:23 <smspillaz> I think that the filename is actually inteded to be an override
16:23 <smspillaz> so I might just check for filename size
16:24 <RAOF> Is there a desperate need for compiz's sounds to be configured differently to the sound theme?
16:25 <RAOF> Hm. Actually, the gtkdoc doesn't make it clear whether or CA_PROP_EVENT_ID *overrides* CA_PROP_MEDIA_FILENAME…
16:26 <RAOF> I guess empirical testing may be in order. Worse than a superfluous configuration option is a superfluous configuration option that *doesn't even work* :)
16:28 <RAOF> smspillaz: So, it's possible my concern is actually ‘the option to set the bell sound filename doesn't work’ :)
16:30 <RAOF> Either it works, and compiz won't follow the sound theme, or it doesn't, and compiz gains a non-functional option. Score!
16:32 <smspillaz> RAOF: we just don't se CA_PROP_MEDIA_FILENAME if mFilename is empty
16:33 <RAOF> smspillaz: And you may need to *not* set CA_PROP_EVENT_ID if mFilename is not empty. http://0pointer.de/lennart/projects/libcanberra/gtkdoc/libcanberra-canberra.html#ca-context-play suggests that it'll override MEDIA_FILENAME.
16:34 <RAOF> But relying on this crazy documentation thing is probably a bit ambitious. There should be actual testing to confirm that libcanberra works that way.
16:35 <RAOF> And I suspect that this testing could be asked of our intrepid branch submitterc.
16:36 <RAOF> Shall I paste this IRC snippet in as a review comment?
16:37 <smspillaz> sure
16:37 <smspillaz> I'll probably just fix it myself though

So - feel free to either resolve my concerns there, or wait for Sam to fix it!

Finally got a chance to look at this.

The documentation was ambiguous when I originally made the plugin, so as
suggested here, I decided simply to test out all the options. I can confirm
that if you set CA_PROP_EVENT_ID as "bell" and don't specify a file name
that it uses the correct bell sound. Furthermore, 'bell-window-system'
actually doesn't work. Since 'bell' itself does, there's no need to specify
the file name, as far as I can tell (it seemed to be overriding the
CA_PROP_EVENT_ID setting).

I'm making the change and pushing it with the filename business entirely
removed.

On Mon, Aug 1, 2011 at 23:44, Chris Halse Rogers <email address hidden> wrote:

> The proposal to merge
> lp:~hypodermia/ubuntu/oneiric/compiz/fix-for-bug-301174 into
> lp:ubuntu/compiz has been updated.
>
> Status: Needs review => Work in progress
>
> For more details, see:
>
> https://code.launchpad.net/~hypodermia/ubuntu/oneiric/compiz/fix-for-bug-301174/+merge/64632
> --
>
> https://code.launchpad.net/~hypodermia/ubuntu/oneiric/compiz/fix-for-bug-301174/+merge/64632
> You are the owner of
> lp:~hypodermia/ubuntu/oneiric/compiz/fix-for-bug-301174.
>

--
Emily Strickland
<email address hidden>
706-363-0950

Actually, this appears to break on my system. I must've had the old bell file cached for that event. I don't see how to make this work as it's documented to supposed to work. We may need to roll back this latest change and, as RAOF said, go with the option where "it works, and compiz won't follow the sound theme".

Libcanberra bug?