Created attachment 290765 [details] [review]
TuneIn plugin

Review of attachment 290765 [details] [review]:

I'd personally have written that in Lua, but hey ;)

You'll need to add a test case for this plugin as well, to test the behaviour of the plugin, and make it easier to reproduce bugs. See tests/bliptv for a good example of that for a plugin that does browse/search.

::: configure.ac
@@ +179,3 @@
 AC_SUBST(GDBUS_CODEGEN)
 
+VALA_CHECK_PACKAGES([grilo-0.2 grilo-net-0.2 json-glib-1.0], HAVE_VALA=yes, HAVE_VALA=no)

I'd prefer the HAVE_VALA variable to be called something else, in case we get another plugin that depends on a slightly different set of libraries. HAVE_TUNEIN_VALA?

::: src/tunein/grl-tunein.vala
@@ +23,3 @@
+                                     MetadataKey.BITRATE,
+                                     MetadataKey.URL,
+                                     MetadataKey.INVALID);

You're probably missing tags here, at least. You should also try to ask designers for a logo to show in the plugin.

@@ +85,3 @@
+        }
+
+        media = new_media_from_type (o.get_string_member ("type"));

We try to be complete, so you should register new metadata when one that doesn't exist could be used to record metadata that the search gives you.

For example, location and language.

@@ +96,3 @@
+
+        if (o.has_member ("URL")) {
+            var url = o.get_string_member ("URL");

Looks like there could be multiple URLs, I'd give the preferred one first (likely the 128kb AAC one) but still add the others to the metadata.

@@ +107,3 @@
+        if (o.has_member ("subtext")) {
+            var subtext = o.get_string_member ("subtext");
+            media.set_description (subtext);

This overwrites the description?

@@ +113,3 @@
+            var item = o.get_string_member ("item");
+            if (audio != null)
+                audio.set_genre (item);

You can add multiple genres, and it seems that some stations have multiple genres (this one seems to have multiple ones, not sure about the search output: http://tunein.com/radio/BBC-Radio-4-935-s25419/)

@@ +162,3 @@
+    }
+
+    public override void browse (Grl.SourceBrowseSpec bs)

Needs to handle count and skip

@@ +190,3 @@
+    }
+
+    public override void search (Grl.SourceSearchSpec ss)

Ditto.

@@ +195,3 @@
+
+        var cancellable = new GLib.Cancellable ();
+        request.begin ("http://opml.radiotime.com/Search.ashx?query=%s".printf (ss.text),

Why is the search at a different host from the service itself?

I'd document that fact, and move the URLs as defines at the top of the file.

Add to the "request" component.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/grilo/issues/53.