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 703619 - Create sources for each optical media
Create sources for each optical media
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: plugins
git master
Other Linux
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2013-07-04 15:24 UTC by Bastien Nocera
Modified: 2014-01-20 13:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
optical-media: Better debug output (3.63 KB, patch)
2014-01-10 13:41 UTC, Bastien Nocera
committed Details | Review
optical-media: Ignore non video media (1.19 KB, patch)
2014-01-10 13:41 UTC, Bastien Nocera
reviewed Details | Review
optical-media: Ignore non video media (1.19 KB, patch)
2014-01-20 11:00 UTC, Bastien Nocera
none Details | Review
optical-media: Ignore non video media (1.19 KB, patch)
2014-01-20 11:02 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2013-07-04 15:24:30 UTC
Rather than creating a container that has them all (as Juan requested from the beginning...)
Comment 1 Bastien Nocera 2013-12-20 17:18:28 UTC
I don't actually think that's possible. A GrlSource isn't a GrlMedia, and can't be a GrlMedia...
Comment 2 Juan A. Suarez Romero 2013-12-24 16:38:09 UTC
I was thinking on a source that provides just one element element: the media itself.

For CD-Audio, maybe it could expose each track as a separated GrlMedia. For DVD, I guess it should be just one media, the DVD itself.
Comment 3 Bastien Nocera 2014-01-10 13:41:18 UTC
(In reply to comment #2)
> I was thinking on a source that provides just one element element: the media
> itself.

We already do that. We show a GrlMedia for each of the optical drive (or loopback-mounted images).

Try this by loop back mounting a video DVD ISO, and a distribution ISO[1]. We'd show 2 entries, one for each of the "optical" devices. The attached patch will remove the distribution ISO from the list.

> For CD-Audio, maybe it could expose each track as a separated GrlMedia. For
> DVD, I guess it should be just one media, the DVD itself.

We currently ignore audio CDs, but it would be pretty straight forward to have them be added as top-level items as well. I don't think we want to show individual items, as a music application probably already has other ways to get metadata on the audio CD that's going to be better than what we can do here.

[1]: If you get a crash in udisks, it's probably https://bugs.freedesktop.org/show_bug.cgi?id=72956
Simply launch "Disks" to have it work.
Comment 4 Bastien Nocera 2014-01-10 13:41:45 UTC
Created attachment 265917 [details] [review]
optical-media: Better debug output

So that we can debug problems a bit more easily.
Comment 5 Bastien Nocera 2014-01-10 13:41:49 UTC
Created attachment 265918 [details] [review]
optical-media: Ignore non video media
Comment 6 Juan A. Suarez Romero 2014-01-20 08:10:51 UTC
(In reply to comment #3)
> We already do that. We show a GrlMedia for each of the optical drive (or
> loopback-mounted images).
> 

My point was having a config option so if you have 3 medias, instead of showing one source with 3 entries, show 3 sources each one with one entry.
Comment 7 Juan A. Suarez Romero 2014-01-20 08:39:40 UTC
Comment on attachment 265917 [details] [review]
optical-media: Better debug output

commit 3baf4e8a3808ac494f440c890b63a5cb1a9c89b4
Author: Bastien Nocera <hadess@hadess.net>
Date:   Fri Jan 10 14:32:56 2014 +0100

    optical-media: Better debug output
    
    So that we can debug problems a bit more easily.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703619

 src/optical-media/grl-optical-media.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
Comment 8 Juan A. Suarez Romero 2014-01-20 08:50:51 UTC
Review of attachment 265918 [details] [review]:

::: src/optical-media/grl-optical-media.c
@@ +419,3 @@
 
+  scheme = g_uri_parse_scheme (uri);
+  if (scheme == NULL || !g_str_equal (scheme, "file"))

If scheme is NULL, is it correct to include as-it-is as URL? URL expects a scheme, so if scheme is NULL either we don't add it or we add a scheme.
Comment 9 Bastien Nocera 2014-01-20 11:00:59 UTC
Created attachment 266715 [details] [review]
optical-media: Ignore non video media
Comment 10 Bastien Nocera 2014-01-20 11:02:48 UTC
Created attachment 266716 [details] [review]
optical-media: Ignore non video media
Comment 11 Bastien Nocera 2014-01-20 11:08:40 UTC
(In reply to comment #6)
> (In reply to comment #3)
> > We already do that. We show a GrlMedia for each of the optical drive (or
> > loopback-mounted images).
> > 
> 
> My point was having a config option so if you have 3 medias, instead of showing
> one source with 3 entries, show 3 sources each one with one entry.

What's the use for this? Apart from working around the "content-changed" signal, I'm not sure.
Comment 12 Juan A. Suarez Romero 2014-01-20 11:22:44 UTC
(In reply to comment #11)
> What's the use for this? Apart from working around the "content-changed"
> signal, I'm not sure.

You would have exactly one source per a physical disk. 

Maybe at this moment is not a big winning, because you know each entry in the optical media is already a physical disk. I think depending on how your application is designed, could make things easier to have exactly one different source per device.

In any case, the suggestion was a reply for the first comment, about how I was thinking on having one source per device in this plugin.
Comment 13 Bastien Nocera 2014-01-20 12:05:22 UTC
I still don't see what it brings that couldn't be done in the application. The application would need to special case the optical-media plugin already so that the presentation is less sucky than it currently is. So a little more special casing isn't going to hurt.
Comment 14 Juan A. Suarez Romero 2014-01-20 12:14:25 UTC
Yes, I was thinking specially on the case where a device could provide several elements (like the CD-Audio showing each track as a separated media; but I agree with you it is better to let the application to handle this).

But as right now I can't think on any real case for one device with several elements, I agree with as it is.
Comment 15 Juan A. Suarez Romero 2014-01-20 12:17:03 UTC
Comment on attachment 266716 [details] [review]
optical-media: Ignore non video media

commit 496c2dd6d34ac2875916268d3436345173844805
Author: Bastien Nocera <hadess@hadess.net>
Date:   Fri Jan 10 14:34:12 2014 +0100

    optical-media: Ignore non video media
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703619

 src/optical-media/grl-optical-media.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Comment 16 Juan A. Suarez Romero 2014-01-20 12:18:36 UTC
Can then we close this bug?
Comment 17 Bastien Nocera 2014-01-20 13:13:53 UTC
Sure thing.