[Bug 725221] dashdemux: Implement RFC 3986 compliant URL joiner

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=725221
  GStreamer | gst-plugins-bad | git

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #282251|none                        |needs-work
             status|                            |

--- Comment #19 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-09-05 08:57:50 UTC ---
Review of attachment 282251:
 --> (https://bugzilla.gnome.org/review?bug=725221&attachment=282251)

Looks very good in general, just some minor comments

::: gst/gsturi.c
@@ +909,3 @@
+ * using the algorithm described in RFC3986.
+ *
+ * Last reviewed on never ()

Remove that part :)

@@ +929,3 @@
+ * fragment NULL.
+ *
+ * Returns: A new GstUri object. [transfer full]

No need to add gtk-doc documentation for private functions

@@ +1458,3 @@
+ * escaped except where indicated.
+ *
+ * Returns: (transfer full): A new #GstUri object.

Add Since: 1.6 at the end of the gtk-doc comments of every public function

@@ +1778,3 @@
+{
+  const gchar *r_scheme;
+  GstUri *t;

Add guards to *all* public functions, e.g. here

g_return_val_if_fail (base_uri != NULL, NULL);
g_return_val_if_fail (ref_uri != NULL, NULL);

@@ +2433,3 @@
+ * gst_uri_set_query_table:
+ * @uri: (transfer none)(nullable): The #GstUri to modify.
+ * @query_table: (transfer none)(nullable): The new query table to use.

Not sure if a GHashTable is ideal for this. Are the GI annotations to tell
about the key and value types?

Maybe we need a GstUriQueryElement struct, and then here use a GList of
GPtrArray of them?

@@ +2560,3 @@
+ * Get a list of the query keys from the URI.
+ *
+ * Returns: (transfer full)(element-type gchar*): A list of keys from

This is "transfer container" as the gchar* don't have to be freed

::: gst/gsturi.h
@@ +173,3 @@
+ */
+typedef struct _GstUri GstUri;
+struct _GstUri

Make this struct private and put it into the .c file. If any direct accesses
are needed, add getters (and setters?) for them

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.