After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 774830 - uri: Add new uri API to get URI fragment as table
uri: Add new uri API to get URI fragment as table
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-11-22 08:01 UTC by Seungha Yang
Modified: 2016-12-06 20:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
uri: Add new uri API to get URI fragment as table (6.26 KB, patch)
2016-11-22 08:02 UTC, Seungha Yang
needs-work Details | Review
uri: Add new uri API to get media fragments URI as table (6.68 KB, patch)
2016-11-23 01:01 UTC, Seungha Yang
committed Details | Review

Description Seungha Yang 2016-11-22 08:01:40 UTC
As an usecase of URI fragment, it can indicate temporal or spatial
dimension of a media stream. To easily parse key-value pair,
newly added gst_uri_get_fragment_table () API will provide
the table of key-value pair likewise URI query.
Comment 1 Seungha Yang 2016-11-22 08:02:34 UTC
Created attachment 340490 [details] [review]
uri: Add new uri API to get URI fragment as table
Comment 2 Seungha Yang 2016-11-22 08:12:51 UTC
MPEGDASH requires URI fragment based MPD Anchor feature.
For example, any mpds start play from 10 Sec if URI fragment is set "t=10"

Not only for MPEGDASH, Media Fragments URI 1.0 (https://www.w3.org/TR/media-frags/) is saying that URI fragment can set some playback features using URI.
For the future generic usage of it in Gstreamer, I think this API is needed.
Comment 3 Sebastian Dröge (slomo) 2016-11-22 11:49:17 UTC
Review of attachment 340490 [details] [review]:

::: gst/gsturi.c
@@ +2822,3 @@
+ */
+GHashTable *
+gst_uri_get_fragment_table (const GstUri * uri)

As this is not defined in the URI spec itself, this should specifically mention what it's doing and based on.

Maybe something like get_media_fragment_table()? Should also mention the link to the spec in the docs.

@@ +2828,3 @@
+  if (!uri->fragment)
+    return NULL;
+  return _gst_uri_string_to_table (uri->fragment, "&", "=", TRUE, TRUE);

Is that really enough? I didn't read the spec but that seems a bit simple :)
Comment 4 Seungha Yang 2016-11-22 13:08:53 UTC
(In reply to Sebastian Dröge (slomo) from comment #3)
> Review of attachment 340490 [details] [review] [review]:

Thanks for your attention of my report :)

> Maybe something like get_media_fragment_table()? Should also mention the
> link to the spec in the docs.

get_media_fragment_table() is more correct expression. Note that, how query "key-value" should be parsed is not defined by URI spec, though.

> @@ +2828,3 @@
> +  if (!uri->fragment)
> +    return NULL;
> +  return _gst_uri_string_to_table (uri->fragment, "&", "=", TRUE, TRUE);
> 
> Is that really enough? I didn't read the spec but that seems a bit simple :)

If only "Media Fragments URI 1.0" is concerned, it's enough. Actually, GstUri's "URI query" is doing exactly same as this.
Comment 5 Seungha Yang 2016-11-23 01:01:03 UTC
Created attachment 340569 [details] [review]
uri: Add new uri API to get media fragments URI as table

- Change API name to gst_uri_get_media_fragment_table()
- Add some comment about what is doing and how.
Comment 6 Tim-Philipp Müller 2016-11-23 10:30:43 UTC
Isn't returning a hash table going to be awkward for bindings?
Comment 7 Seungha Yang 2016-11-23 11:52:23 UTC
(In reply to Tim-Philipp Müller from comment #6)
> Isn't returning a hash table going to be awkward for bindings?

Can I get some more information about "be awkward for bindings", please?

If your intention was concerning for a kind of case that 
"what happen if non-Media Fragment URI but just URI Fragment was parsed and converted to hash table?", my answer is that unexpected "key-value" pair will be returned. 

For example, "http://foo/var/file#&&=afragment" will be converted to
key = "" (null string) and value = "afragment".

This API cannot guarantee the "validity" of returned "key-value" pair (such as null key value case), but just do split of URI fragment string based on "&" and "=". For unknown extended standard that might be somewhere, It is doubtful whether it should be validated.
As an expanded usecase what I know for [&, = driven "key-pair" rule] is MPEGDASH. It's defining "period" and "as" as a keys but which are undefined keys by Media Fragments URI 1.0.
Comment 8 Tim-Philipp Müller 2016-11-23 13:53:17 UTC
I was talking about gobject-introspection and python bindings and such.
Comment 9 Seungha Yang 2016-12-05 09:44:18 UTC
(In reply to Tim-Philipp Müller from comment #8)
> I was talking about gobject-introspection and python bindings and such.

Sorry for late replay. Since I has no experiance about gobject-introspection or pygobject, it's difficault to answer your question... But, they seems to support GHashTable when I googling about them.
Comment 10 Sebastian Dröge (slomo) 2016-12-05 10:16:45 UTC
Hash tables should be fine, and it's what is used in the API for similar things already.
Comment 11 Sebastian Dröge (slomo) 2016-12-05 10:17:37 UTC
Review of attachment 340569 [details] [review]:

Looks good to me under those considerations. Tim?
Comment 12 Tim-Philipp Müller 2016-12-06 20:30:01 UTC
Oh, ok, I missed the fact that bindings can handle this already and that there are annotations for it, sorry!

commit 0494c173e0b72005146195ca001771a0cd8eb5ba
Author: Seungha Yang <sh.yang@lge.com>
Date:   Tue Nov 22 16:52:46 2016 +0900

    uri: Add new uri API to get media fragments URI as table
    
    As an usecase of URI fragment, it can indicate temporal or spatial
    dimension of a media stream. To easily parse key-value pair,
    newly added gst_uri_get_media_fragment_table () API will provide
    the table of key-value pair likewise URI query.
    See also https://www.w3.org/TR/media-frags/
    
    https://bugzilla.gnome.org/show_bug.cgi?id=774830