GNOME Bugzilla – Bug 629002
Add GRL_METADATA_KEY_SIZE
Last modified: 2014-03-19 11:32:32 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.
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...
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).
We also give that as metadata in totem-pl-parser.
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.
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).
(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.
Created attachment 272311 [details] [review] tracker: Export file size
Created attachment 272313 [details] [review] pls: Set file size attribute
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.
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.
The grl_media functions invoke grl_data, which have the guards. Do we need to add guards to grl_media too?
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.
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.
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.
Attachment 272313 [details] pushed as b88a996 - pls: Set file size attribute Attachment 272370 [details] pushed as 09d9b0a - core: Add metadata key for size
Attachment 272311 [details] pushed as 5a487a5 - tracker: Export file size