Re: [Rhythmbox-devel] Sources, Types and Errors

[Jonathan Matthew <jonathan kaolin wh9 net>]

On Fri, Dec 22, 2006 at 10:45:00PM +0100, Tom Parker wrote:
> 
> Lots of posting from me today :-) This is what having some bored time over Xmas does to me...
> 
> I noted a while back that all import errors from all sources are
> visually dumped under the Library
> (http://bugzilla.gnome.org/show_bug.cgi?id=369415). I've been fishing
> through the Rhythmbox sources in order to try and fix this, and
> realised it was a much bigger problem than I thought, and so I'd like
> to ask a few questions about why things are how they are, and whether
> they should be changed.
> 
> Specifically, the relationship between Sources, EntryTypes and errors.
> As I understand it, in general each Source has a single EntryType that
> it does stuff with. It looks like it's possible to have more than one
> EntryType per source, but I can't see that being used. Then, if we
> have an import error of some sort, that's a separate EntryType
> entirely.

The entry-type property of a source is really only used inside the
source to help it construct queries.  Externally, there's a mapping from
entry types to sources, but this is only used for some minor UI purposes.

The set of entries that appear in a source in the UI is not determined
by the source's entry-type property.  It's determined by the criteria of
a query model that the source creates, which may or may not filter based
on the entry type.

> This (to me) appears to have a couple of problems, but it mainly comes
> down to "All Sources get told about all items, irregardless of their
> EntryTypes". Given that each Source generally has it's own EntryType,
> and it's been associated with that Source, I can't see why things are
> done this way. Firstly, it's wasteful that every Source needs to do
> filtering. 

The filtering required is a simple as adding an entry type criterion to
a query.  This amounts to a pointer comparison.

> Secondly, this means that the Library doesn't do any
> filtering on errors, and just shoves anything with an error type into
> the Import Errors dialog irregardless of whether it came from a
> GenericPlayer source or wherever.

So, there are three ways to separate import errors from generic players
out:
- multiple import error entry types
- filter on path name (painful for the library import error source)
- use the same entry type for import errors and normal files
  (complicates matters somewhat)

I was planning to fix this using the first approach.  The second would
work, but it would mean the query model for the library's import error
source would get complicated - we'd need to add 'path doesn't start with
/media/sda1' criteria to it to keep out import errors from portable
players.

Import errors are substantially different to normal entries.  Mostly,
they cannot be played and they should not be saved to the on-disk
database.  We've already got special cases in various places for hidden
songs (missing files), which mostly makes sense because songs can change
between being hidden and not fairly regularly, and changing an entry
type is quite complicated.  Adding more special cases for import errors,
when transitions between error and not-error state are extremely rare,
isn't the right approach.

> IMHO, entries should always only be reported back to the Source they
> came from, and errors should just be entries marked as bad rather than
> separate generic "one type fits all Sources" EntryType.

How would playlists work if we did this?  They use the same
entries that appear in the library.