Play sounds for new notifications

[nightly-brew]

Some notification daemons support playing sounds when a new notification pops up, some don't. The xdg notification spec even describes hints for controlling the sound behaviour, such as suppress, specify an xdg-sound-name to play or directly pass the path of a file.

[nightly-brew]

In the weekend I should be able to setup a new branch and play around with the code. I'll probably use libcanberra, but I also saw that GSound and Gstreamer are available in case good 'ol canberra is unsuitable.

[ErikReider]

I think that elementary users canberra for audio so it should be doable :)

[nightly-brew]

Working on it right now, sorry for the delay, had a busy weekend. Should dnd affect the sound as it does for the notiWindow?

[ErikReider]

Oh nw! Thanks for the help! Dnd shouldn't play any sounds

[nightly-brew]

What about urgent notifications? They pop up even when dnd is enabled

[ErikReider]

Hmmm. I'm not sure about that one. A config option would be good though

[nightly-brew]

So like DND on android, where you can have urgent visual notifications turned on, but sound disallowed right? If so, I'm almost suggesting we give the option to completely hide even urgent notifications.

[nightly-brew]

I don't think I see a predefined precedence for sounds defined in the spec. What if a notification happens to have both sound-file and sound-name defined?

I think only sound-file should be played, since the client might have defined it to play a special sound. Maybe sound-name should be picked up as a fallback if the above file cannot be played, for whathever reason?

[ErikReider]

File over name sounds like the best solution. Should we play a default sound of there is none specified? Maybe another config option?

[nightly-brew]

File over name sounds like the best solution. Should we play a default sound of there is none specified? Maybe another config option?

Sure! Unless, for any reason, suppress-sound is specified: in that case no sound is played.

I'd also play the default sound if the sound-name is not recognised, unless canberra already takes care of it.

[nightly-brew]

Ok, work is almost done, there's just a couple of things to iron out.

Apparently libcanberra, if provided with both sound name and sound file, chooses first the named sound, then the sound file; we kinda inverted the behaviour, is it still good? I would like to make it behave similar to other daemons, to avoid possible confusion.

What should we do about the default sound? At the moment, it is hardcoded to the named sound "message-new-instant": this means the sound changes upon installing a different sound theme following the relevant xdg spec. We could provide a setting to change the default named sound to something else, like "message-new-email" of "dialog-error" for example.

There's also the possibility of allowing users to set a default sound file, which supposedly could be put as a first fallback, while leaving the named sound fallback as last resort, but 4 tries to just play a bling seem a little overkill IMHO. See the player code here for clarifications on this last statement

Thoughts?

[ErikReider]

The default sound file could get pretty messy so I'd stick with a configurable sound name as a fallback.

As mentioned in another issue, I think that the spec includes notification categories like mail and social so a different sound could be set for each

[nightly-brew]

I need to fiddle a bit with categories first, but I should be done tonight.

[ErikReider]

https://github.com/ErikReider/SwayNotificationCenter/blob/0f69f080b40d3621069bfadea0a414ba2e96bb3a/src/notiModel/notiModel.vala#L154-L158 We should probably convert this into an enum :)

[nightly-brew]

Thanks for the suggestion! If anything else needs to be ported, but is not related to this issue, I'm gonna open new ones ;)

[nightly-brew]

mhm, categories are specified as class.specific_case, like network.disconnected. In case a non standard category like network.banana is passed, should we try to pick network (as it exists in this case) or just drop to the default category?

Enums are flat, but maybe a conversion function string hint -> enum could split on the dot and pick the class first.

[nightly-brew]

Nevermind, x-vendor.class.name would create problems... (and it's allowed)

We could decide to not support them and just fallback to the default category, or try and implement something like nested enums..? I'm not even sure if vala has them.

[ErikReider]

We should stick with the standardized categories and add some gnome / KDE specific ones too just to be safe

[nightly-brew]

Just to be sure, at this point, would you support specifyng a sound for every subclass or just for classes?

Like, would you want network.error to be different from network.connected or would you just use network?

[ErikReider]

Hmmm. What about using the same error sound for all errors?

Something like this

case "network.error":
case "something.error":
    // play error sound
    break;

[nightly-brew]

Some of my toughts on these proposals (I'm also proposing another solution):

1. support every vendor.class.subclass by specifying a variable in the config:
   • gets big pretty fast (cons)
   • allows fine grained control of category sounds (pro)
   • if handled tree-like (not flat), allows falling back gracefully (pro)
2. support only vendor.class by specifying a variable in the config:
   • not as big as the first case and still explicitly declared in config (pro)
   • network error and network connected would have the same sound, which is confusing (cons)
3. group errors and others subclasses together:
   • doesn't allow for fine grained control of sounds (cons)
   • there aren't many common groups in the spec as of now, although we could consider deviced.added, transfer.complete and other successful actions simply as "success" (kinda cons IMHO)
4. search for vendor.class.subclass, vendor.class or class.subclass audio files in a predefined directory:
   • category - sound is not explicit in the config file (cons)
   • at the same time, it allows to slim down the relevant code (pro)
   • is pretty similar to xdg-sound-theme spec, which we decided is a little bit messy to be relied upon for defaults (cons)

[nightly-brew]

Maybe I have a solution which could get all the pros of the above one.

Suppose we can get at max a 3 level deep tree, as it seems to be the case in the spec. Each sound could be identified in nested dicts, vendor -> class -> subclass. With this structure xdg classes could go under xdg or freedesktop vendor, while others like gnome, kde and the likes would get their fancy group.

Each class could have an entry named "default" just for the sake of having something play if a notification only ships with the category class.

Users could provide their preference without us putting in any new code just to support a specific sound if we allowed them to be written in nested objects in the config, like this:

"sounds": {
  "xdg": {
    "network": {
      "default": "network.ogg",
      "connected": "network-connected.ogg"
    },
    "email": {
      "default": "email.ogg"
    }
  },
  "gnome": {
    "screenshot": {
      "error": "gnome-screenshot-error.ogg"
    }
  }
}

This way, configs get bigger, not the code itself, and the solution is more generic (at least it sounds so to me, correct if I'm wrong).

I suppose the json scheme should be flexible enough to allow this king of structures, right?

[nightly-brew]

We could also do the same thing for showing category-based images, as you proposed in #70

[mrlectus]

I was just thinking about this when I saw an issue was already raised. good job guys

[nightly-brew]

Woke up with an idea to simplify my last proposal.

First of all, the config could be written without nested structure by reusing the "dot notation" from xdg.

"sounds": {
  "network.connected": "netconn.ogg",
  "network.disconnected": "netdisc.ogg",
  "email": "maildefault.ogg",
  "email.new": "newmail.ogg",
  "gnome.screenshot.error": "screenerror.ogg"
}

This is smaller and certainly plain. Parent group - child element rations are also somewhat clear by the notation itself.

On the code side, nested structures are also droped in favor of only one dict which uses the vendor.class.subclass strings as keys.

This allows to do a direct lookup when a notification first comes up, while with yesterday night's proposal had to decompose the category string by splitting on the dots.

Furthermore, if the category is found, the sound file is already on our hands, else the noti category string is modified by removing the last ".something" portion from it. After that, the new string is searched again in the dictionary.

Repeat until a sound is found or no match is detected, which leads to the usage of a fallback or default sound.

vendor.class.subclass vendor.class vendor ""

In case the vendor is not specified, like with standard proposal classes

class.subclass class ""

This loop does at most 3 cycles.

[ErikReider]

That sounds like a good plan! But to future proof this I have a suggestion. Instead of specifying a sound, we could add a dict for sounds, images, icons, etc... The icons would ofc not be added by this pr

[nightly-brew]

That would certainly prevent deduplication. Something like this then?

"categories-settings": {
  "network.connected": {
    "sound": "netconn.ogg",
    "icon": "icon-network-conn"
  },
  "network.disconnected": {
    "sound": "netdisc.ogg",
    "icon": "icon-network-disc"
  },
  "email": {
    "sound": "maildefault.ogg",
    "icon": "letter"
  },
  "email.new": {
    "sound": "newmail.ogg"
  },
  "gnome.screenshot.error": {
    "sound": "screenerror.ogg",
    "icon": "ui-dialog-error"
  }
}

With a structure like this one, it would be better to associate a category class to the key.

[ErikReider]

That looks really good. Makes it easier to add more features in the future if needed :)

[nightly-brew]

Good! I'm proceeding this way then

[nightly-brew]

yet another halt... Vala doesn't have native dictionaries support.

We could add yet another dependency, libGee, or we could use a custom made structure.

I figured if we use a list to hold the cats, sorted so sibling items go side by side, the lookup shouldn't be too much expensive. I'm not a big fan of this solution to be honest, but is doable.

[ErikReider]

Can't you use GLib.HashTable?

[nightly-brew]

I can! I just didn't know it was there, my bad...

When I tried searching here, nothing came up, so I was led to the wrong conclusion, and a quick search on internet only brought up libGee, not GLib.

Thanks!

[ErikReider]

Yeah, documentation is a little lacking... I usually use valadoc

[nightly-brew]

Gonna use it from now on ;)

[nightly-brew]

I can't seem to get Json.gobject_deserialize to work, it throws "Json-Message: Invalid value of type 'GHashTable' " when it gets to this new part:

/** Categories settings */
private GLib.HashTable<string, Category?> _categories;
public GLib.HashTable<string, Category?> categories {
    get {
        _categories = _categories ?? new GLib.HashTable<string, Category?> (str_hash, direct_equal);
        return _categories;
    }
}

Category is a struct

public struct Category {
    string sound_file { get; set; }
    string icon_name { get; set; }
}

What am I doing wrong?

edit: I tried converting Category from a struct to a class, thinking maybe gobject_deserialize doesn't like structs. I now get another warning in conjunction: (swaync:154285): Json-WARNING **: 15:38:48.321: Failed to deserialize "categories" property of type "GHashTable" for an object of type "SwayNotificatonCenterConfigModel"

Still no progress though

[ErikReider]

Hmmm. Seems like json-glib doesn't support deserializing into hashmaps/hashtables...

[nightly-brew]

I'm now trying with a simbple object and at the moment I'm having the same difficulties...

[ErikReider]

We'll probably need to manually deserialize the object using the deserialize_property override

[ErikReider]

Managed to kinda get it working. Here's the diff: https://gist.github.com/ErikReider/5a6a539f8e32058ffbd77265d395f107 Here's the raw code:

        /** Categories settings */
        public HashTable<string, Category> categories {
            get;
            set;
            default = new HashTable<string, Category>(str_hash, str_equal);
        }
        public bool categories_settings {
            private get;
            set;
            default = false;
        }

        /* Methods */

        public override bool deserialize_property (string property_name,
                                                   out Value value,
                                                   GLib.ParamSpec pspec,
                                                   Json.Node property_node) {
            switch (property_name) {
                case "categories-settings" :
                    print ("FOUND %s\n", property_node.type_name ());
                    value = true;
                    extract_categories (property_node);
                    return true;
                default:
                    // Handles all other properties
                    return default_deserialize_property (
                        property_name, out value, pspec, property_node);
            }
        }

        private void extract_categories (Json.Node property_node) {
            // Sets categories variable...
        }

[ErikReider]

The public bool categories_settings is used as a dummy so that deserialize_property detects and tries to set that property

[nightly-brew]

I should have managed to get rid of the dummy property, but I'll know after I compile the code

[nightly-brew]

Deserialization support landed! It was horrible, geez there's not so much content about vala online, that or duckduckgo hates me and my search queries.

https://github.com/nightly-brew/SwayNotificationCenter/commit/28cfc8f76dfe62eaeb09e22577089deda96e8842

Json.gobject_deserialize still complains about an unsupported hash parameter in the class, but It can be safely ignored I think at this point. I altered your gist so that if the real categories-settings deserialization fails badly, it gets picked up by the Parser.

It's not the best of code and a gentle hand from a more skilled programmer could really help, but it seems to do its things.

Little todo, probably:

• add the serialization counterpart
• test if reloading the config works or not
• update the json schema

[nightly-brew]

Oh, I was almost forgetting.

In the json file, the property is "categories-settings", but in the configModel the real property is just categories.

[nightly-brew]

I may be crazy, but that implementation doesn't satisfy me. I'm going to try and make the hashtable serializable, this way we don't need all that glue.

https://stackoverflow.com/questions/43344017/vala-serializing-object-property-with-json-gobject-serialize and https://github.com/major-lab/json-api-glib/blob/master/src/json-api-object.vala are making me hopeful it will work!

[ErikReider]

I may be crazy, but that implementation doesn't satisfy me. I'm going to try and make the hashtable serializable, this way we don't need all that glue.

You madlad! If you need any help, just ask! :)

[nightly-brew]

Well if you know more than me on vala introspection and the works, since Json-GLib requires ParamSpec...

Anyway I'm half-there, I wasn't able to directly extend HashTable since inheritance is disabled for it (flike for a final class in Java, I suppose). I created a SerializableDict which keeps a table internally and is accessible through proxy methods, and as of now Json serialization works in half.

[nightly-brew]

With the code provided in those links, I'ma able to serialize a 3 cats dictionary in this way, which is a bit confusing...

{
  "t-type" : {
    "key1" : {
      "icon-name" : "icon-cat-1"
    },
    "key3" : {
      "icon-name" : "icon-cat-3"
    },
    "key2" : {
      "icon-name" : "icon-cat-2"
    }
  },
  "t-dup-func" : {
    "key1" : {
      "icon-name" : "icon-cat-1"
    },
    "key3" : {
      "icon-name" : "icon-cat-3"
    },
    "key2" : {
      "icon-name" : "icon-cat-2"
    }
  },
  "t-destroy-func" : {
    "key1" : {
      "icon-name" : "icon-cat-1"
    },
    "key3" : {
      "icon-name" : "icon-cat-3"
    },
    "key2" : {
      "icon-name" : "icon-cat-2"
    }
  }
}

It's probably picking too much, but I'm still investigating how to stop that.

[nightly-brew]

I'm onto something, I think

{
  "$schema" : "/etc/xdg/swaync/configSchema.json",
  "positionX" : "center",
  "positionY" : "top",
  "control-center-margin-top" : 0,
  "control-center-margin-bottom" : 0,
  "control-center-margin-right" : 0,
  "control-center-margin-left" : 0,
  "timeout" : 10,
  "timeout-low" : 5,
  "timeout-critical" : 0,
  "notification-window-width" : 500,
  "keyboard-shortcuts" : true,
  "image-visibility" : "when-available",
  "transition-time" : 200,
  "hide-on-clear" : false,
  "hide-on-action" : true,
  "dnd-play-urgent-sound" : true,
  "categories-settings" : {
    "test.subclass" : {
      "sound-file" : "soundfile.ogg",
      "icon-name" : "icon-test"
    }
  },
  "array-test" : [
    "element_one",
    "element_two"
  ],
  "array-test2" : [
    {
      "icon-name" : "test-cat-array"
    }
  ],
  "cat" : {
    "icon-name" : "icon-baby",
    "sound-file" : "file1.ogg"
  }
}Json-Message: 19:06:19.316: Invalid value of type 'GHashTable'
Json-Message: 19:06:19.316: Invalid value of type 'GStrv'
Json-Message: 19:06:19.316: Invalid value of type 'SwayNotificatonCenterCategory'
element_one0icon-baby

key1
holds type SwayNotificatonCenterCategory
valid paramspec name: true
key3
holds type SwayNotificatonCenterCategory
valid paramspec name: true
key2
holds type SwayNotificatonCenterCategory
valid paramspec name: true

(swaync:899292): GLib-GObject-WARNING **: 19:06:19.316: invalid unclassed pointer in cast to 'GParam'

(swaync:899292): Json-CRITICAL **: 19:06:19.316: json_serializable_get_property: assertion 'G_IS_PARAM_SPEC (pspec)' failed
serializing key1
holds type (null)
valid paramspec name: true

(swaync:899292): GLib-GObject-WARNING **: 19:06:19.316: invalid unclassed pointer in cast to 'GParam'

(swaync:899292): Json-CRITICAL **: 19:06:19.316: json_serializable_get_property: assertion 'G_IS_PARAM_SPEC (pspec)' failed
serializing key3
holds type (null)
valid paramspec name: true

(swaync:899292): GLib-GObject-WARNING **: 19:06:19.316: invalid unclassed pointer in cast to 'GParam'

(swaync:899292): Json-CRITICAL **: 19:06:19.316: json_serializable_get_property: assertion 'G_IS_PARAM_SPEC (pspec)' failed
serializing key2
holds type (null)
valid paramspec name: true
{
  "key1" : {
    "sound-file" : "random.ogg",
    "icon-name" : "icon-cat-1"
  },
  "key3" : {
    "icon-name" : "icon-cat-3"
  },
  "key2" : {
    "icon-name" : "icon-cat-2"
  }
}

I somewhat successfully (even though there's a barricade of errors...) serialized this:

Category cat1 = new Category ();
cat1.icon_name = "icon-cat-1";
cat1.sound_file = "random.ogg";

Category cat2 = new Category ();
cat2.icon_name = "icon-cat-2";

Category cat3 = new Category ();
cat3.icon_name = "icon-cat-3";

SerializableDict<Category> dict = new SerializableDict<Category> ();
dict.insert ("key1", cat1);
dict.insert ("key2", cat2);
dict.insert ("key3", cat3);

stderr.printf ("\n\n");
stderr.printf (Json.to_string (Json.gobject_serialize (dict), true));
stderr.printf ("\n\n");

[ErikReider]

I think that we should move the serializable hashtable into it's own PR. It'll make the sound PR a little smaller and easier to follow :)

[nightly-brew]

Right :)