Re: [gfvs] cdda backend

[David Zeuthen <david fubar dk>]

Hi,

On Mon, 2007-12-17 at 10:16 +0100, Alexander Larsson wrote:
> I'm not convinced this is what people want (over launching e.g.
> sound-juicer or gnome-cd when clicking on the audio disc icon), 

Yeah, I wasn't really clear about that. 

The main points are

 a. to get the apps to use the same IO path (gio) instead of every one
    of them using cdparanoia or similar crufty API's. 

 b. having a centralized IO path allows us to handle the case where an
    app tries to access the drive directly when you're burning a disc;
    the cdda:// backend will just flat out refuse to mount the
    disc unless there is audio on it (and it knows, a'priori, via hal
    if there is: e.g. no IO is needed to find out).

 c. a small (but vocal) minority of us likes to use the command line
    and non-gnome POSIX apps so have the files available via ~/.gvfs
    is nice

I'm pretty sure we're all in violent agreement about that looking at
cdda:// in the file manager is a terrible experience for casual users
(e.g. my mom). 

My proposal here is to use a Clue bar [1] to tell the user to use
sound-juicer or Rhythmbox or whatever the preferred app for CD audio is
(more on preferred apps in a future mail). Probably just bring the user
directly to the app if they double click the disc. 

So users ever only get to the file manager view if they select it in the
file chooser or the file browser sidebar. And even there they get a
Cluebar.

[1] : Credit goes to mclasen for coining the term Clue bar (= the
firefox style bar in Nautilus that is shown for both trash: and burn:)

> but lets
> add it, it doesn't hurt.

OK, I'll commit it after cleaning it up a bit more.

> > First, a small feature list
> > 
> >  - Stateful mounts; yes, you actually "mount" the cdda gvfs filesystem
> >    driver; there's one instance of gvfsd-cdda per mount. You also have
> >    to mount the fs manually: gvfs-mount cdda://sr0
> 
> You don't actually have to make each cdda mount its own process. If you
> want you can add a well-known dbus name to the cdda.mount.in file (which
> is missing from your patch btw). gvfsd will hand off cdda mount
> operations to that name if it exists, instead of spawning a new copy.

But I actually want a separate process for each mounted disc... seems
easier if I don't have to care about life cycle management at all (e.g.
when unmounted gvfsd will terminate the backend process).

> >  - Works with multiple clients at the same time as well as when
> >    a iso9660 filesystem is mounted by a kernel driver. There's seeking
> >    and it's a bit slow (can be alleviated once we introduce readahead
> >    in the GVfsBackend base class, yes?) but basically works very well
> 
> Readahead is mainly gonna speed up the case where an app does
> read_source()
> write_destination()
> read_source()
> write_destingation()
> ...
> 
> I.e. while it blocks writing what it just read we can automatically
> schedule a read from the source so that the next read will finish
> faster. Its not really related to seeks.

It's indirectly related; the thing is that each read operation will get
to read more sectors before the other ones. Thus less seeking is
involved, thus higher throughput.

> >  2. I have to set GMountSpec in try_mount(); it's not enough just
> >     setting it in do_mount(). Bug?
> 
> Yes, that shouldn't happen. Can you try to look into this?

Will do.

> >  3. Once mounted, there's no way for the backend to change it's display
> >     name. I think I understand why you did this; it's because of the
> >     FUSE daemon right? E.g. you want to ensure that there are no
> >     name collisions to make it easier to figure out what directories
> >     to return in ~/.gvfs ...
> > 
> >     Anyway, the reason we want this is that initially we probably want
> >     the display name "Audio Disc" and if we (much later) acquire meta
> >     data for the disc we want to change it to "Tori Amos - Silent
> >     all these years" or whatever the title is.
> > 
> >     (same applies for the icon (think cover art); I haven't tested if
> >      this doesn't work)
> 
> Hmmm. Tricky. Its kinda weird having the name of the actual mountpoint
> change. Especially with it being the identifier for the mount in the
> fuse case. I think the best solution is to have the volume be called
> _("Audio Disc"), but then set the display name of the root of the disk
> to the actual disk title. Thats what will be displayed in many places
> like the window title for the toplevel directory.

Maybe we want

 g_vfs_backend_set_display_name (backend, display_name);
 g_vfs_backend_set_local_mount_name (backend, display_name); [1]

where the latter can't change once mounted and is used by the FUSE
daemon. The latter can change as much as we want (AFAICT it's fine to
have collisions in displayable names for both GDrive, GVolume and
GMount.)

[1] : If not apparent already, I suck at naming. Feel free to suggest
alternatives

> > This all sounds complicated but I'm not sure it really is. I think
> all
> > we need to do is to have some kind of tags we can decorate the
> > GHalVolume and GDaemonVolume objects so they can find each other.
> > Anyway, you probably have some ideas I haven't thought about...
> 
> I don't see an obvious easy solution. It requires some work. The ideal
> thing to match with is an GMountSpec (which is basically the set of
> key-value pairs that define a gvfs mount). For a GDaemonMount this is
> availible in the GMountInfo object.
> 
> Thats a gvfs-specific object though, so we can't make that availible in
> the base GVolume apis. What if we add some new vtable calls in
> GVolumeMonitor like find_volume_for_new_mount() and
> find_mount_for_new_volume() which we have GUnionVolumeMonitor call on
> the installed backends when new, unowned volume/mounts are added.

Yeah, that sounds pretty good. There's some details on how to uniquely
identify the volume (using display_name is not enough; uuid may be if we
can guarantee that one is available. Not sure.). I'll look into this and
will send a patch.

> Some comments on the patch:

Thanks for the review and the pointers on how to fix these; I'll do
that.

      David