Code review comment for lp:~hypodermia/ubuntu/oneiric/compiz/fix-for-bug-301174

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!