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 629002 - Add GRL_METADATA_KEY_SIZE
Add GRL_METADATA_KEY_SIZE
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: core
git master
Other Linux
: Normal enhancement
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2010-09-07 22:26 UTC by Juan A. Suarez Romero
Modified: 2014-03-19 11:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
core: Add metadata key for size (5.12 KB, patch)
2014-03-03 00:05 UTC, Juan A. Suarez Romero
needs-work Details | Review
tracker: Export file size (1.05 KB, patch)
2014-03-18 17:03 UTC, Bastien Nocera
committed Details | Review
pls: Set file size attribute (1.13 KB, patch)
2014-03-18 17:09 UTC, Bastien Nocera
committed Details | Review
core: Add metadata key for size (5.14 KB, patch)
2014-03-18 17:09 UTC, Bastien Nocera
reviewed Details | Review
core: Add metadata key for size (5.21 KB, patch)
2014-03-19 11:17 UTC, Bastien Nocera
committed Details | Review

Description Juan A. Suarez Romero 2010-09-07 22:26:45 UTC
This new key would contain the size of media, if available.

Very useful for Internet content, so clients can know in advance how much will it weight before downloading it.
Comment 1 Bastien Nocera 2010-11-20 23:50:12 UTC
Wouldn't the clients figure that out when opening the file anyway? Makes no sense for streaming (RTSP, RTP, MMS), and could be gathered from HTTP as well. My guess is that it's only useful for broken HTTP servers...
Comment 2 Juan A. Suarez Romero 2010-11-22 08:28:23 UTC
Yes, clients can know it when opening the file (or downloading it). But the goal is that applications can expose it without the user required to open the file to know the size.

Of course, not all plugins will handle this key. One of the use-case I thought is a server which provides hundred of (non-streaming) media files. I, as a user, would like to know how much each of them weights, so I can decide if I want to download some of them now (If I have time) or let it for later (if I'm in a hurry).
Comment 3 Bastien Nocera 2012-03-27 15:15:04 UTC
We also give that as metadata in totem-pl-parser.
Comment 4 Juan A. Suarez Romero 2014-03-03 00:05:22 UTC
Created attachment 270736 [details] [review]
core: Add metadata key for size

Add GRL_METADATA_KEY_SIZE, and proper functions, that get/set the size in media
size in bytes.
Comment 5 Bastien Nocera 2014-03-15 12:40:55 UTC
Review of attachment 270736 [details] [review]:

Do you also have a patch for grl-pls to export that info for local files?

::: src/data/grl-media.c
@@ +1130,3 @@
+grl_media_set_size (GrlMedia *media, gint size)
+{
+  grl_data_set_int (GRL_DATA (media),

This is missing guards
g_return_if_fail (GRL_IS_MEDIA (media));

@@ +1815,3 @@
+grl_media_get_size (GrlMedia *media)
+{
+  return grl_data_get_int (GRL_DATA (media), GRL_METADATA_KEY_SIZE);

Ditto.

::: src/grl-metadata-key.c
@@ +515,3 @@
+                                           g_param_spec_int ("size",
+                                                             "Size",
+                                                             "Episode of the media",

Copy/paste error?

@@ +516,3 @@
+                                                             "Size",
+                                                             "Episode of the media",
+                                                             0, G_MAXINT,

I'd use -1 as "unknown", 0 is a reasonable size (though not too useful).
Comment 6 Juan A. Suarez Romero 2014-03-17 22:52:56 UTC
(In reply to comment #5)
> Review of attachment 270736 [details] [review]:
> 
> Do you also have a patch for grl-pls to export that info for local files?
> 

No, it should be added.

> ::: src/data/grl-media.c
> @@ +1130,3 @@
> +grl_media_set_size (GrlMedia *media, gint size)
> +{
> +  grl_data_set_int (GRL_DATA (media),
> 
> This is missing guards
> g_return_if_fail (GRL_IS_MEDIA (media));
> 
> @@ +1815,3 @@
> +grl_media_get_size (GrlMedia *media)
> +{
> +  return grl_data_get_int (GRL_DATA (media), GRL_METADATA_KEY_SIZE);
> 
> Ditto.

Yes, the patch is before applying the one about adding guards. I'll add them.

> 
> ::: src/grl-metadata-key.c
> @@ +515,3 @@
> +                                           g_param_spec_int ("size",
> +                                                             "Size",
> +                                                             "Episode of the
> media",
> 
> Copy/paste error?
> 

O:-)

> @@ +516,3 @@
> +                                                             "Size",
> +                                                             "Episode of the
> media",
> +                                                             0, G_MAXINT,
> 
> I'd use -1 as "unknown", 0 is a reasonable size (though not too useful).

Makes sense.
Comment 7 Bastien Nocera 2014-03-18 17:03:37 UTC
Created attachment 272311 [details] [review]
tracker: Export file size
Comment 8 Bastien Nocera 2014-03-18 17:09:20 UTC
Created attachment 272313 [details] [review]
pls: Set file size attribute
Comment 9 Bastien Nocera 2014-03-18 17:09:44 UTC
Created attachment 272314 [details] [review]
core: Add metadata key for size

Add GRL_METADATA_KEY_SIZE, and proper functions, that get/set the size in media
size in bytes.
Comment 10 Bastien Nocera 2014-03-18 17:12:41 UTC
I see that we didn't add the guards in the functions in that file yet, so I didn't add them to the patch.
Comment 11 Juan A. Suarez Romero 2014-03-19 10:15:12 UTC
The grl_media functions invoke grl_data, which have the guards. Do we need to add guards to grl_media too?
Comment 12 Juan A. Suarez Romero 2014-03-19 10:18:20 UTC
Review of attachment 272314 [details] [review]:

::: src/data/grl-media.c
@@ +1815,3 @@
+grl_media_get_size (GrlMedia *media)
+{
+  return grl_data_get_int (GRL_DATA (media), GRL_METADATA_KEY_SIZE);

As it is, it will return 0 if the media doesn't contain a size.

If we want to return -1 in this case, we need to check if the media contains the key and if not return -1, else return the key value.
Comment 13 Juan A. Suarez Romero 2014-03-19 10:21:07 UTC
Bastien, if you prefer to return -1 in case size is unknown, just address my previous comment. Else, feel free to push all the patches.
Comment 14 Bastien Nocera 2014-03-19 11:17:38 UTC
Created attachment 272370 [details] [review]
core: Add metadata key for size

Add GRL_METADATA_KEY_SIZE, and proper functions, that get/set the size in media
size in bytes.
Comment 15 Bastien Nocera 2014-03-19 11:28:12 UTC
Attachment 272313 [details] pushed as b88a996 - pls: Set file size attribute
Attachment 272370 [details] pushed as 09d9b0a - core: Add metadata key for size
Comment 16 Bastien Nocera 2014-03-19 11:32:28 UTC
Attachment 272311 [details] pushed as 5a487a5 - tracker: Export file size