shotwell

Here is a suggestion on how to fix this:

On startup, when validating the Shotwell database, if it doesn't exist (i.e. Shotwell is running for the first time), then:

• Check whether an F-Spot database exists (~/.config/f-spot/photos.db),

• Check that the version of the F-Spot database is supported by the importer (meta table),

• Should support at least DB versions in Ubuntu Lucid (17) and Maverick (??)

Check that the F-Spot database is not empty,

If yes, open a dialogue box asking the user if he wants to import the existing F-Spot database.

Add a menu item such as File -> Import F-Spot Database so that users can decide to import later.

If the user decides to import:

• Import the content of the tags table:

• The F-Spot special category Events should map to Shotwell events,

• What should we do with other F-Spot special categories: Favourites, Hidden, People, Places?

Import the content of the photos, photo_tags and photo_versions table,

What should be done with the rolls table?

Ignore the exports and jobs tables.

The F-Spot import should use the same progress feedback as the import from file or camera.

If that sounds sensible, I'm happy to take ownership of this bug and provide an initial fix.

Replying to [comment:6 seb1204]:

In the light of the upcoming Ubuntu release 10.10 Maverick I would consider importing from F-Spot a very much wanted/needed feature.

Here's the link to the blueprint https://blueprints.launchpad.net/ubuntu/+spec/desktop-maverick-shotwell

Initial patch attached. So here's what works so far:

• Discovered F-Spot databases appear in the sidebar in the same way as cameras,

• Selecting the DB displays all photographs held in it (same behaviour as camera import),

• Import works but only handles photo files, no tags, events or DB-held metadata supported,

• Automatic DB version discovery, with support for DB v16 (as in Ubuntu Jaunty) and v17 (as in Lucid); only tested with v17 though.

What needs to be done next:

• It seems that on Jaunty, the DB is held in ~/.gnome2/f-spot/photos.db rather than ~/.config/f-spot/photos.db so the code should handle this,

• Handle tags, events and other DB-held metadata,

• Add a way to unmount an external database (and possibly give the user the ability to ignore that DB in the future),

• Most of the code in AlienDatabaseImportPage is adapted from ImportPage, it would be good to re-factor this code to extract the common code between both into a super-class,

• Add a “Mount external database” menu option to allow the user to browse to a .db file that is in an unusual location,

• Add support for more versions of the F-Spot database.

For people who want to help on the last point, can you make sure you have sqlite3 installed, run the following and attach the output to this bug please:

$ cd ~/.config/f-spot

$ sqlite3 photos.db

sqlite> .schema

sqlite> select * from meta;

sqlite> ^D

from one of my backup DBs

~/.config/f-spot$ sqlite3 photos.backup.2009-08-18.db

SQLite version 3.6.22

Enter ".help" for instructions

Enter SQL statements terminated with a ";" 

sqlite> .schema

CREATE TABLE exports (id INTEGER PRIMARY KEY NOT NULL, image_id         INTEGER NOT NULL, image_version_id INTEGER NOT NULL, export_type      TEXT NOT NULL, export_token     TEXT NOT NULL);

CREATE TABLE jobs (

id INTEGER PRIMARY KEY NOT NULL,

job_type         TEXT NOT NULL,

                                job_options      TEXT NOT NULL,

                                run_at           INTEGER,

job_priority     INTEGER NOT NULL

);

CREATE TABLE meta (idINTEGER PRIMARY KEY NOT NULL,nameTEXT UNIQUE NOT NULL,dataTEXT);

CREATE TABLE photo_tags (        photo_id      INTEGER,           tag_id        INTEGER,           UNIQUE (photo_id, tag_id) );

CREATE TABLE photo_versions (          photo_id        INTEGER,        version_id      INTEGER,        name            STRING,    uriSTRING NOT NULL,md5_sumSTRING NOT NULL,protectedBOOLEAN);

CREATE TABLE photos ( id                 INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,   time               INTEGER NOT NULL,      uri   STRING NOT NULL,   description        TEXT NOT NULL,           roll_id            INTEGER NOT NULL,   default_version_id INTEGER NOT NULL,   rating   INTEGER NULL,   md5_sum   TEXT NULL     );

CREATE TABLE rolls (                            id          INTEGER PRIMARY KEY NOT NULL,         time        INTEGER NOT NULL   );

CREATE TABLE tags (                            id            INTEGER PRIMARY KEY NOT NULL,       name          TEXT UNIQUE,                        category_id   INTEGER,          is_category   BOOLEAN,          sort_priority INTEGER,          icon          TEXT   );

CREATE INDEX idx_photo_versions_id ON photo_versions(photo_id);

CREATE INDEX idx_photos_roll_id ON photos(roll_id);

sqlite> select * from meta;

1|F-Spot Version|0.5.0.3

2|F-Spot Database Version|16.3

3|Hidden Tag Id|2

as I forgot to ask…

How I understand the patch description the F-Spot DB will be 'mounted' in Shotwell. Shotwell will then works with the photos in the F-Spot DB until unmounted.

Will the Photos and in future the tags be transferred/merged into Shotwell's DB?

Another thing regarding F-Spot would be to recognize F-Spot written Metadata in photos on importing them into Shotwell. But this should be a different bug, correct?

Bruno,

thanks very much for the initial iteration of your patch. I just tried it out and was able to successfully import some photos from F-Spot into Shotwell, so this is quite promising. I think this will likely end up being the most substantial external (i.e. non-Yorba) contribution to Shotwell so far!

Jim and I are not yet completely sold on having the F-Spot database show up in the Shotwell side bar – we're not convinced the import feature needs to be this fancy. It's a clever idea, but I think we'd actually be quite happy with an implementation where the user simply chooses File->Import From->F-Spot… and can perform the import in a dialog. That might be substantially simpler to implement, because you wouldn't need to worry about displaying the photos from the F-Spot database in any way. It's true that a sidebar implementation would allow the user to select only a subset of photos to import. But such an implementation will always display one flat list of all the photos in the F-Spot database, and there might be many thousands, so it might be quite hard for the user to find any given photo(s) that they want. (By the way, a user can already drag selected photos from the F-Spot window into the Shotwell window directly to import them, and their metadata will also be imported if the user has checked the F-Spot option to store metadata inside the image files.)

So, to summarize, we think that a plainer implementation which simply uses a dialog might be easier to implement and might well be good enough, especially since most users will perform this task only once. Even if your goal is ultimately to have an integrated sidebar implementation, it might be nice to first implement the import using a dialog, get that completely working and then work on the fancier integration later; that would reduce risk and would allow us to address some of the fundamental problems (e.g. importing more metadata from F-Spot) sooner.

Anyway, that's our initial reaction. Let us know what you think, and please keep up the good work!

Adam,

I agree with you that this will probably be done only once so could be done via a File -> Import From -> F-Spot menu item. What I originally tried was to have an automatic import on first start but didn't manage to get that to work properly so decided to have a look at how the camera import worked and derive my code from that.

At the end of the day, the most complex part is not the UI code, it's the import and database layer code. So I suggest that I keep the UI as it is for now (it works even though it's not the most elegant code in the world) and concentrate on getting the tags and events sorted, as well as improving the F-Spot database support.

Bruno,

OK – I didn't realize that the user interface was potentially temporary. It seems like a reasonable idea to continue working on the import functionality itself (tags, events, F-Spot database versions). Once that's all working, we can talk more about what the user interface should look like. Thanks again for all your help with Shotwell!

We're planning to implement 1-5 star ratings for 0.7 (see#2233, #2234, #2235, #2236), so these should be imported from F-Spot as well.

Adam,

That's no problem: reading the rating from the F-Spot database is straightforward. I subscribed myself to bug #2233 so that as soon as it is implemented, I can include it into the F-Spot import code. If a nice set_rating(int) method is added to LibraryPhoto, it should be a one-liner :-)

Speaking of Bruno, as we've shipped 0.6, my time loosened a bit. If you want to throw me another patch for a look-over, I'd be happy to send you feedback.

Replying to [comment:20 jim]:

Speaking of Bruno, as we've shipped 0.6, my time loosened a bit. If you want to throw me another patch for a look-over, I'd be happy to send you feedback.

Thanks for the offer. The new patch is nearly ready. I just need to do some basic testing to make sure it doesn't fall over horribly and I'll send it your way.

Additional specification from Adam sent to the mailing list to deal with ratings:

Considering that F-Spot includes, a 1-5 star rating, a hidden flag and a favorite tag, what should be the behaviour when an F-Spot photo is imported that is hidden, has a rating of 3 stars and has the favorite tag attached? Does the rating take precedence?

I'd suggest that if an F-Spot photo has a numeric rating (3 stars in this case), we should use that. If it has no rating, but is a favorite, let's give it 5 stars. If it has no rating, but has the hidden flag, let's assign it to be Rejected. If it has no rating and is both hidden and a favorite, let's just make it unrated (the default value).

A second version of the patch is attached. New stuff in this patch:

• Tags now get imported, including parent tags;

• The code should support any version of the F-Spot database from v0 to v18 (latest one).

One aspect that I'm currently struggling with is importing events. The idea was to add events in the same way as tags, i.e:

• Look event by name,

• If it doesn't exist, create an empty event,

• Associate the new photo with the event.

So I added a name_map to the Event class, like the Tag class does but it doesn't seem to work as expected. Also, it doesn't seem to check whether automatically generated events (dates) already exist and it creates new ones every time, which means you may end up with several events for the same date. I'll try to work out what's going wrong but any help in that area would be welcome.

Other things that don't yet work (or at least not knowingly):

• No handling of different versions of a given photo;

• Hidden and favourite flag import is untested.

In terms of design, here is a quick explanation of what the different additional files do:

• !AlienDatabaseImportPage.vala: all the UI is here and is independent of the database used so could be re-used for other import functions; it would also be the file to change if we wanted to move the import function to a menu item.

• !AlienDatabase.vala: defines a number of interfaces to be implemented by any alien database driver.

• FSpotDatabaseDriver.vala: an implementation of the interfaces defined in AlienDatabase.vala for the F-Spot database. It also includes the core logic to select a set of specific behaviours depending on the version of the database found.

• FSpotDatabaseTables.vala: the low level implementation of the F-Spot table and behaviour objects that actually pull the data out of the DB.

Hi Bruno,

Looking over the code, I like how this is shaping up. Using the complete() method in BatchImportJob solved a lot of problems that I thought were going to be inside BatchImport; you put them where they should be, on the importers shoulders, which seems right to me.

And very happy to hear that this code works with all F-Spot database versions; that's absolutely fantastic.

To answer some of your questions:

• I need to clarify what an Event is in the Shotwell world. An Event is not merely all pictures taken on a particular day (that is, a kind of chronological filter of your library). Events are generated at import time from that particular import roll. Thus, if you plug in one camera and import and plug in another and import, and all the photos on those cameras were taken on the same day, you will get two Events, both named 7 July, 2010 (for example). This is by design. If you think about events being “New Year's Eve” and “Birthday Party” and not strictly tied to time-of-exposure, this makes some sense.

From my tests, the Event generation is working as designed when importing from F-Spot. The only thing missing is applying an F-Spot event name to the proper Shotwell event. Since events in F-Spots are more like our tags, it may be that all photos tagged “Event / London” in F-Spot will reside in one or more Shotwell events all named “London” -- something to that effect.

• I did notice that you're searching in some well-known locations for the F-Spot database. Makes sense, to be sure, but I do know that F-Spot allows the user to (from the command-line) specify the location of the database. We might need to allow the user to locate it themselves from the UI … don't worry about this now, but it's something we'll need to consider later (which is why I'm recording it here).

• No worries about different versions of a given photo; we'll present them as separate photos to the user.

• Hidden and Favorite flags are on their way out to make way for five-star ratings (see #2233). Specifically, we are going to have 1-5 stars as well as two additional states: Unrated and Rejected. Hidden will map to Rejected and Favorite will map to 5 stars. I think the F-Spot import should work likewise.

Unfortunately, you can't really work on this until the patch is landed. This is something we might have to add later (before 0.7 ships, of course).

• I've looked over your code and am happy not to see any glaring problems or oversights. You're following Yorba's coding conventions (which is important to us) and everything seems neatly factored and organized. As mentioned before, we're still thinking about the UI aspect of it (hopefully we'll have more concrete details soon), but what you have today at least allows us to test out the meat of this patch.

All in all, very happy about what I'm seeing here. Hopefully the above comments can help you get at some of the rough corners.

Replying to [comment:24 jim]:

And very happy to hear that this code works with all F-Spot database versions; that's absolutely fantastic.

Well, that's the theory behind the code. In practice, I have only tested with v17 so far (Lucid). So that assumption will need to be validated.

• I need to clarify what an Event is in the Shotwell world. An Event is not merely all pictures taken on a particular day (that is, a kind of chronological filter of your library). Events are generated at import time from that particular import roll. Thus, if you plug in one camera and import and plug in another and import, and all the photos on those cameras were taken on the same day, you will get two Events, both named 7 July, 2010 (for example). This is by design. If you think about events being “New Year's Eve” and “Birthday Party” and not strictly tied to time-of-exposure, this makes some sense.

From my tests, the Event generation is working as designed when importing from F-Spot. The only thing missing is applying an F-Spot event name to the proper Shotwell event. Since events in F-Spots are more like our tags, it may be that all photos tagged “Event / London” in F-Spot will reside in one or more Shotwell events all named “London” -- something to that effect.

OK, that makes sense. This means that in the complete() method, I need to find the automatically generated event and rename it accordingly. And I'll also need to make sure that if I have 2 photos taken on the same day but associated with 2 different F-Spot events it does the right thing.

• I did notice that you're searching in some well-known locations for the F-Spot database. Makes sense, to be sure, but I do know that F-Spot allows the user to (from the command-line) specify the location of the database. We might need to allow the user to locate it themselves from the UI … don't worry about this now, but it's something we'll need to consider later (which is why I'm recording it here).

Yes I noticed that the location can vary so allowing the user to browse the file system to find the database will be needed at some point.

• No worries about different versions of a given photo; we'll present them as separate photos to the user.

Fine. What I meant in my comment is that at the moment the code doesn't handle the F-Spot photo_versions table at all so I'm not sure it finds versions of a given photo other than the original one. I need to test this and add the relevant code if it doesn't work.

• Hidden and Favorite flags are on their way out to make way for five-star ratings (see #2233). Specifically, we are going to have 1-5 stars as well as two additional states: Unrated and Rejected. Hidden will map to Rejected and Favorite will map to 5 stars. I think the F-Spot import should work likewise.

Unfortunately, you can't really work on this until the patch is landed. This is something we might have to add later (before 0.7 ships, of course).

Yes, this is why I haven't bothered testing them at all.

• I've looked over your code and am happy not to see any glaring problems or oversights. You're following Yorba's coding conventions (which is important to us) and everything seems neatly factored and organized. As mentioned before, we're still thinking about the UI aspect of it (hopefully we'll have more concrete details soon), but what you have today at least allows us to test out the meat of this patch.

All in all, very happy about what I'm seeing here. Hopefully the above comments can help you get at some of the rough corners.

Good, I'm glad you're happy about it. Your comments are very helpful. I think that the biggest hurdle I face at the moment is handling the event generation properly so this is what I will concentrate on for the next version of the fix.

Bruno,

I also tried out your latest patch and this continues to look very promising. Great work so far!

Jim and I finally got the chance to meet and talk about F-Spot importing in detail. Here's our detailed proposal about how this should work. If anything here doesn't make sense to you then of course we're happy to discuss more.

• As I'm sure you know, F-Spot allows multiple versions of each photo. The original version of each photo is called “Original”. If the user makes edits to a photo, F-Spot creates a version called “Modified”. The user can also create additional versions. F-Spot displays a dropdown list where the user can select the current version of each photo, i.e. the version that displays in the main browser view. That selection persists across F-Spot sessions.

• Shotwell allows only 2 on-disk versions of each photo: the master copy and the editable copy. When you use Shotwell's Open in External Editor command, Shotwell creates the editable copy and the external editor is invoked on it. Shotwell's Revert to Original command will remove the editable copy and revert to the master.

• If a photo in F-Spot has only a single version (“Original”) then that should simply become a single master photo in Shotwell. You've already implemented this. :)

• If a photo in F-Spot has multiple versions, and the current version is not the Original version, then the current version should become the editable copy in Shotwell and the Original version should become its master copy in Shotwell. Also, we'd like to import any other versions as separate photos which will appear side-by-side in Shotwell along with the current version. We'd ideally like to make additional copies of the Original version to serve as master copies for these additional versions. (The copies are necessary because Shotwell will not allow a single on-disk photo to server as a master for several different photos in the library). If that's tricky, however, we'd be OK with simply importing the additional versions as master photos rather than as master/editable pairs.

• If a photo in F-Spot has multiple versions, and the current version is the Original version, then we'd like to import all versions as separate photos in Shotwell. Again, ideally we'd make additional copies of the Original version to serve as master copies for each of the additional versions.

• Here's an example. Suppose that a photo in F-Spot has two versions, Original and Modified, and that Modified is current. Then we'll import this as a single photo: Modified becomes the editable copy and Original is the master. If, however, Original is current, then we'll import this as two photos: one displays the Original, which is its own master, and the other displays the Modified copy, which is ideally an editable copy whose master is a new on-disk copy of the Original photo.

I hope that all of this so far is clear. If you think that any of this is too complicated or too hard to implement then let us know and we can discuss more.

• F-Spot lets the user specify comments for each photo. We'd like these comments to become the photo's title in Shotwell.

• As you know, F-Spot has hierarchical tags but Shotwell does not (yet). We like the way you're importing both parent and child tags into Shotwell. Note, however, that F-Spot creates a parent tag called “Import Tags” used to hold all tags automatically generated by reading metadata from imported photos. We think it's not terribly useful to have this tag show up in Shotwell and so we'd prefer to exclude it.

• Jim and I are now not convinced that it makes sense to try to generate Shotwell events from tags under F-Spot's Event tag. One fundamental problem is that a photo in F-Spot may be tagged with several different Event tags, but in Shotwell each photo can belong on only one event. Also, Shotwell events are intended to hold photos all taken at nearly the same time, but we're not sure that F-Spot users will always be using F-Spot Event tags in this way. So I think we'd actually be happy to treat F-Spot's Event tag in the same way as any other F-Spot tag.

• As you know, Shotwell cannot display import rolls. The underlying Shotwell database does, however, store an import ID for each photo, so we can potentially implement an import roll feature at any time using the existing data in Shotwell databases. Given this, I think we would actually like to import F-Spot's import roll information into the Shotwell database, even though this will not yet be displayed in any useful way to the user.

• With your changes, I tried importing the same set of photos twice and saw that the second import had no effect: the already-imported photos were simply ignored. That's great.

• As I suggested in an earlier comment on this ticket, I think we probably don't want to keep your existing view that shows thumbnails for all the photos in the F-Spot database. There are several reasons for this, one of which is that that view won't be terribly useful to users once they've migrated from F-Spot. Given this, we'd like to have a command File->Import From F-Spot… that performs this import. When the user chooses that command, we'd like to display a dialog that says something like this: “472 photos found in F-Spot library. Import these photos now? [Cancel] [Import]”.

• Also, when the user first runs Shotwell we may ask whether they want to import from F-Spot at that time. Jim and I are still contemplating the design of the dialog that will appear on first run, and so you don't need to worry about that – we will take case of both designing and implementing that.

OK – so that's all for now. Obviously we'd welcome your feedback. If you think that this may be too much to implement in the time we have, then please let us know and we can discuss how to either scale this back or to have some Yorba engineers help out with the project. Once again, we deeply appreciate all your help building this important feature!

Replying to [comment:26 adam]:

Adam,

I'll have a look at how I can handle versions as you suggest. I will start with loading all versions as individual photos, make sure it works and add the tweaks afterwards. Now, regarding your other comments:

• F-Spot lets the user specify comments for each photo. We'd like these comments to become the photo's title in Shotwell.

That's fine.

• As you know, F-Spot has hierarchical tags but Shotwell does not (yet). We like the way you're importing both parent and child tags into Shotwell. Note, however, that F-Spot creates a parent tag called “Import Tags” used to hold all tags automatically generated by reading metadata from imported photos. We think it's not terribly useful to have this tag show up in Shotwell and so we'd prefer to exclude it.

I was also considering not importing the “Places”, “Events” and “People” top-level tags themselves, as they would just clutter the tag list. What do you think?

• Jim and I are now not convinced that it makes sense to try to generate Shotwell events from tags under F-Spot's Event tag. One fundamental problem is that a photo in F-Spot may be tagged with several different Event tags, but in Shotwell each photo can belong on only one event. Also, Shotwell events are intended to hold photos all taken at nearly the same time, but we're not sure that F-Spot users will always be using F-Spot Event tags in this way. So I think we'd actually be happy to treat F-Spot's Event tag in the same way as any other F-Spot tag.

Good, that will simplify the code!

• As you know, Shotwell cannot display import rolls. The underlying Shotwell database does, however, store an import ID for each photo, so we can potentially implement an import roll feature at any time using the existing data in Shotwell databases. Given this, I think we would actually like to import F-Spot's import roll information into the Shotwell database, even though this will not yet be displayed in any useful way to the user.

Do you mean that I should import the F-Spot roll ID as the Shotwell import ID? If yes, I can do that no problem.

• As I suggested in an earlier comment on this ticket, I think we probably don't want to keep your existing view that shows thumbnails for all the photos in the F-Spot database. There are several reasons for this, one of which is that that view won't be terribly useful to users once they've migrated from F-Spot. Given this, we'd like to have a command File->Import From F-Spot… that performs this import. When the user chooses that command, we'd like to display a dialog that says something like this: “472 photos found in F-Spot library. Import these photos now? [Cancel] [Import]”.

That's fine. I may need help with that part of the code though.

• Also, when the user first runs Shotwell we may ask whether they want to import from F-Spot at that time. Jim and I are still contemplating the design of the dialog that will appear on first run, and so you don't need to worry about that – we will take case of both designing and implementing that.

That's what I tried (and failed) to do at first so if you can get it to work that would be great. To work out whether there are any databases to import, the AlienDatabaseHandler class has a get_discovered_databases function that will return a collection of automatically discovered databases, which you can then use to ask the user whether he wants to import from them.

Replying to Bruno:

Thanks – all of your comments make sense. I agree that loading each F-Spot version as an individual photo makes sense to start out with. We can then start combining versions as I suggested, time permitting.

I agree that we might just skip importing the “Places”, “Events” and “People” top-level tags themselves. One challenge, though, is that presumably these tags (and “Import Tags” as well) may have translated names in non-English builds of F-Spot. Can you recognize them anyway via some fixed ID in the F-Spot world? Or will we need to have Shotwell look for tags with the appropriate translated strings? If this is hard, we could consider importing the tags anyway – I don't think it would be such a tragedy.

I'm not sure that we can simply import the F-Spot roll ID as the Shotwell import ID, because the IDs might conceivably collide. I think the ideal would be to create a new Shotwell import ID for each F-Spot roll ID. This isn't an absolutely necessary feature, but might be nice to have.

You said you might need help with the code that displays the File->Import From F-Spot… dialog. Does that mean that you'd like us to answer questions, or you'd like us to write some of that code? We're open to either, so let us know how you'd like to work together on this.

One important deadline to be aware of: we're planning to freeze strings for Shotwell 0.7 in just two weeks, on Thursday July 29. This F-Spot import code will undoubtedly introduce new strings, so we'd like it to be committed in some form before that date. If you can't implement all the import features I suggested in my message above, by that time, that's completely fine; we can always implement some of them after string freeze (or even for 0.8, possibly). I think our near-term goal should be to implement the File->Import From F-Spot… dialog and get it committed with the import features we have today. Once again, let us know how you'd like to work together on that – we can answer any questions you have, or perhaps Jim could write a skeletal import dialog implementation and you could merge your F-Spot import code into that. Thanks for your continuing help!

To step in here, I also think we should not import the F-Spot import ID's as-is; our ImportID is actually a time_t taken at the start time of the import process. This allows us to (a) easily order the import ID's and find the last one, and (b) display a date/time for the operation. The name is a little misleading, as it looks like a foreign key in the database. (My goof.)

If the F-Spot import ID can be translated confidently into a time_t, then that might be the way to go. If not, I think Adam's right, we should simply generate a new Shotwell ImportID and use that.

Replying to [comment:29 adam]:

I agree that we might just skip importing the “Places”, “Events” and “People” top-level tags themselves. One challenge, though, is that presumably these tags (and “Import Tags” as well) may have translated names in non-English builds of F-Spot. Can you recognize them anyway via some fixed ID in the F-Spot world? Or will we need to have Shotwell look for tags with the appropriate translated strings? If this is hard, we could consider importing the tags anyway – I don't think it would be such a tragedy.

I can identify those tags in a language independent manner as they are the only ones to use a stock icon (and have an icon field that starts “stock_icon:”).

I'm not sure that we can simply import the F-Spot roll ID as the Shotwell import ID, because the IDs might conceivably collide. I think the ideal would be to create a new Shotwell import ID for each F-Spot roll ID. This isn't an absolutely necessary feature, but might be nice to have.

If the F-Spot import ID can be translated confidently into a time_t, then that might be the way to go. If not, I think Adam's right, we should simply generate a new Shotwell ImportID and use that.

In practice, F-Spot does something very similar (with just an additional level of indirection) so I can use the time field in the rolls table to transform the roll ID into a time_t.

You said you might need help with the code that displays the File->Import From F-Spot… dialog. Does that mean that you'd like us to answer questions, or you'd like us to write some of that code? We're open to either, so let us know how you'd like to work together on this.

One important deadline to be aware of: we're planning to freeze strings for Shotwell 0.7 in just two weeks, on Thursday July 29. This F-Spot import code will undoubtedly introduce new strings, so we'd like it to be committed in some form before that date. If you can't implement all the import features I suggested in my message above, by that time, that's completely fine; we can always implement some of them after string freeze (or even for 0.8, possibly). I think our near-term goal should be to implement the File->Import From F-Spot… dialog and get it committed with the import features we have today. Once again, let us know how you'd like to work together on that – we can answer any questions you have, or perhaps Jim could write a skeletal import dialog implementation and you could merge your F-Spot import code into that. Thanks for your continuing help!

The part that will introduce new strings is the UI so I need to have it sorted by that time. I will concentrate on the import dialog during the coming week and will notify you if I have any problem with it.

A third version of the patch is attached. So here is what it includes:

• The UI has changed and now uses a menu item (File -> Import from F-Spot) that opens a small dialog;

• Photo versions are handled in a way that different F-Spot versions of the same photo are imported as different photos;

• F-Spot descriptions are imported as photo titles;

• Ratings are imported, hidden sets the rating to rejected and favorite sets it to 5-star;

• Import ID is imported from the time-stamp in the F-Spot rolls table (and it works quite nicely with the new last import view);

• All tags are imported, except the ones that have a stock icon (Events, Places, Import Tags, etc);

• Events are now imported as tags (but the event code is still present in the generic classes and has been tested to some degree, so it can be used with imports from other databases in the future);

• All translatable strings are now in the code, unless changes are made to the dialog (see below).

Stuff that could be improved:

• Handling of the different versions of a photograph could be smarter, as suggested by Adam 2 weeks ago.

• Someone who is more qualified than I am in terms of usability should have a look at the new dialog: in particular, there is a short amount of time after clicking “OK” where it stands there and does not appear to do anything (in practice, it's building the list of import jobs) so some sort of feedback or progress bar would be useful; or maybe make building the list of import jobs a background job itself.

• If the user has changed the icon associated to a custom tag in F-Spot and has selected a stock icon, rather than a photo, this tag will not get imported. The alternatives are to import all tags (and virtually every single photo will be tagged with “Places”, “Events” and “People”) or to do something a lot more complicated to identify all the tags that F-Spot generates automatically. There is no ideal solution because there is no language independent and reliable way to identify the tags to prune so the current solution seems the most pragmatic.

Quick summary of the new classes:

• Generic files that provide the overall import functionality and are independent of F-Spot:

• !AlienDatabaseImportDialog.vala: the UI dialog that is presented to the user.

• !AlienDatabaseImportJob.vala: the BatchImportJob implementation that does all the hard work.

• !AlienDatabase.vala: the generic code that provides an abstraction layer for different implementations.

F-Spot specific files:

• FSpotDatabaseDriver.vala: the implementation of the interfaces defined in AlienDatabase.vala for F-Spot; it also includes the mechanism to change behavior depending on the database version.

• FSpotDatabaseTables.vala: all the gory details about the F-Spot database tables.

Bruno,

thanks a lot for this latest patch – this seems like a big step forward. I've tried out this patch and my comments are below. I haven't looked at the code – Jim will do that once he's available. Jim is actually travelling to Europe today, and his next working day will be next Tuesday at GUADEC. So on that day I think we can all meet, talk about the patch and he should be able to review it in its state at that time.

So here are my comments, almost all of which are related to the dialog and user interface issues. It would be great if you'd like to work on some of these points before we meet at GUADEC next week.

• The menu item File->Import from F-Spot should end with an ellipsis (“…”) and should be capitalized like this: “Import From F-Spot…”. (See the GNOME interface guidelines: !http://library.gnome.org/devel/hig-book/stable/design-text-labels.html.en#layout-capitalization ).

• This menu item should also have a keyboard mnemonic (i.e. one letter should appear underlined in the menu item).

• In the Import from F-Spot dialog, we should change the title to match the capitalization change above (“Import From F-Spot”).

• The dialog looks a bit crowded. I recommend simplifying the text as follows:

• Import from default F-Spot library (~/.config/f-spot/photos.db)

• Import from another F-Spot database file

• I'm not even sure whether we want to display the path of the default library in this dialog. If so, I think it's import to replace '/home/adam' with '~' to reduce the amount of visible text.

• I think it looks odd to have a horizontal separator between the two radio button elements; let's eliminate that. I think we should also indent the file selector to the right a bit to clearly indicate that it belongs to the second radio button. We should also disable the file selector when the second radio button is not selected. Also, I don't think there's not enough padding between the dialog window edges and the items inside the dialog. Please review the dialog layout guidelines in the GNOME interface guidelines: !http://library.gnome.org/devel/hig-book/stable/design-window.html.en .

• I haven't looked at your code, but it appears that you're not using Glade to lay out the dialog. We're trying to move in the direction of using Glade for all our dialogs, so I think you should do that (and it should make the layout significantly easier as well). Our preferences dialog is constructed using Glade, so you might want to look at that as a model.

• If I choose File->Import from F-Spot, then select a file which is not an F-Spot database file and try to import, I see this:

ERROR:FSpotDatabaseTables.vala:96:fspot_meta_table_get_data: assertion failed: (res == SQLITE_OK)

Aborted (core dumped)

• As you said, there's a short time after clicking OK when the application seems to be hung. This isn't ideal, but I think we can live with this for a first checkin. We can attempt to improve this before the 0.7 release if possible.

• Your pragmatic solution to identifying the tags to prune from the F-Spot import seems like a reasonable compromise.

Replying to [comment:35 adam]:

• The menu item File->Import from F-Spot should end with an ellipsis (“…”) and should be capitalized like this: “Import From F-Spot…”. (See the GNOME interface guidelines: !http://library.gnome.org/devel/hig-book/stable/design-text-labels.html.en#layout-capitalization ).

• This menu item should also have a keyboard mnemonic (i.e. one letter should appear underlined in the menu item).

• In the Import from F-Spot dialog, we should change the title to match the capitalization change above (“Import From F-Spot”).

Done, using F (as in F-Spot) for the keyboard mnemonic.

• The dialog looks a bit crowded. I recommend simplifying the text as follows:

• Import from default F-Spot library (~/.config/f-spot/photos.db)

• Import from another F-Spot database file

• I'm not even sure whether we want to display the path of the default library in this dialog. If so, I think it's import to replace '/home/adam' with '~' to reduce the amount of visible text.

• I think it looks odd to have a horizontal separator between the two radio button elements; let's eliminate that. I think we should also indent the file selector to the right a bit to clearly indicate that it belongs to the second radio button. We should also disable the file selector when the second radio button is not selected. Also, I don't think there's not enough padding between the dialog window edges and the items inside the dialog. Please review the dialog layout guidelines in the GNOME interface guidelines: !http://library.gnome.org/devel/hig-book/stable/design-window.html.en .

I'll look into this. The separator was here to give a bit of breathing space but I agree that it looks odd. Also note that the options are dynamically generated depending on the number and locations of F-Spot databases that the application finds. There are several locations that it could be in and you could have more than one (or none at all), hence why it specifies the location.

Concerning the file selector, the idea was to leave it enabled and automatically select the last option when a file is selected.

• I haven't looked at your code, but it appears that you're not using Glade to lay out the dialog. We're trying to move in the direction of using Glade for all our dialogs, so I think you should do that (and it should make the layout significantly easier as well). Our preferences dialog is constructed using Glade, so you might want to look at that as a model.

You're correct, I'm not using Glade. I'll look into how to do that.

• If I choose File->Import from F-Spot, then select a file which is not an F-Spot database file and try to import, I see this:

> %(=caps)ERROR%:FSpotDatabaseTables.vala:96:fspot_meta_table_get_data: assertion failed: (res == SQLITE_OK)

> Aborted (core dumped)

OK, bad error handling, I'll solve that.

Adam,

A fourth version of the patch is attached that should address all your comments: the dialog is now built with Glade, includes a lot more white space, simpler wording and handles errors gracefully. The last issue is the delay when you click OK. I added a small progress bar to show that something is happening but the dialog doesn't seem to refresh as expected when I switch to it so it never becomes visible, probably because it doesn't yield to the GTK display thread.

I'll see you at GUADEC next week and we can iron out the last remaining issues.

Patch v4 does not work for me, how can i get more debugging information out of shotwell?

Tryed with trunk version 1983 and shotwell-bug-139.4.diff on f-spot version 0.6.1.1

I have about 12.000 photos and about 37000 tags, it does something for about 7 seconds then it fails with the message: “shotwell fail to lead the database”

A tip for the patch, would be cool if after clicking OK to grey out the buttons, otherwise the user won't know that there is something in progress …

Greeting from Beijing

xuedi

Replying to [comment:38 xuedi]:

Patch v4 does not work for me, how can i get more debugging information out of shotwell?

Tryed with trunk version 1983 and shotwell-bug-139.4.diff on f-spot version 0.6.1.1

I have about 12.000 photos and about 37000 tags, it does something for about 7 seconds then it fails with the message: “shotwell fail to lead the database”

Hi Xuedi,

Thanks for trying the patch out and for your feedback. Can you run the following in a terminal window and attach the output to this ticket please?

$ cd ~/.config/f-spot

$ sqlite3 photos.db

sqlite> .schema

sqlite> select * from meta;

I would also appreciate if you could attach the content of the file ~/.cache/shotwell/shotwell.log, as it is just after you attempt to do an import.

A tip for the patch, would be cool if after clicking OK to grey out the buttons, otherwise the user won't know that there is something in progress …

Yes, I am aware of the problem and intend to resolve this in the final release.

Greeting from Beijing

Greetings from London! Good to see that Shotwell is widely used :-)

Bruno,

I've now tried out your latest patch (v4). The dialog is now looking realy good. I now think this is very close, at least from a functionality standpoint. Hopefully Jim will get to review the latest code in the next couple of days. In any case, we'll see each other soon at GUADEC – will look forward to discussing more.

Replying to [comment:39 brunogirin]:

The schema is here: http://pastebin.com/a5VwZt42

sqlite> select * from meta;

1|F-Spot Version|0.6.1.1

2|F-Spot Database Version|17

3|Hidden Tag Id|2

~/.cache/shotwell/shotwell.log

L 5931 2010-07-26 12:21:48 [%(=caps)MSG%] main.vala:69: Verifying database …

L 5931 2010-07-26 12:21:53 [%(=caps)MSG%] FSpotDatabaseDriver.vala:161: Discovered database: /home/xuedi/.config/f-spot/photos.db

Cheers

xuedi

Bruno,

I have tried this patch against my 13Gb collections of photos and everything worked OK. I took ages in my atom netbook but I'm now a happy shotwell user.

Im (was) using f-spot 0.7.1, Database version 18.

I just found a couple of glitches:

- It took a really log time since I pressed OK until the dialog was closed and the import started. It's confusing.

- The import used a huge quantity of memory, more than 800mb that was not freed after the import finished. Shotwell was barely usable until I restarted it.

Anyways, shotwell works really good, Its way more resource friendly that f-spot. The only thing Im missing from f-spot are hierarchical tags ( I know that is tagged for 0.8, will wait).

Just ask if you need more testing, Im happy to help.

Great work.

/Matias

Bruno:

I've done a more thorough look-over of the patch. As I said, we're almost there. A few outstanding things:

• As we talked about today, spin_event_loop() in the dialog box will update the progress bar.

• Make OK button insensitive if a custom database file is selected but the path is set to “None”.

• If the error handling code is easily fixed, would love to see that, but if not, we can ticket that for later.

• Likewise, if the memory consumption problem is easy, great, otherwise, we'll ticket for later.

• Can you look over the strings in the new AlienDatabase and FSpotAlienDatabase code one more time? Specifically, I noticed that some of the exception error strings were not marked as translatable. Are those errors ever being shown to the user (i.e. in an error dialog)?

• I noticed the URI decode code in FSpotURI.from_string(). Without me delving into FSpot's database, is there something odd about their URIs? I'm thinking File.new_for_uri() could solve this problem (assuming all their URI's are file://, which is what I think your code is also assuming).

-- Jim

Patch v5 is attached. This includes the following:

• Modifications to the dialog:

• Immediate feedback is now provided when clicking the OK button,

• The OK button is greyed out if the custom database is selected but the file is set to None,

• The spin_event_loop() fix suggested by Jim;

Simplification of the URI handling code, as suggested by Jim;

Additional cleanup code when the dialog closes.

Jim, the error strings in the AlienDatabase and FSpotDatabase classes are not displayed to the user, they are only logged in the log file, hence why I did not mark them as translatable. And as discussed earlier today, the updated error handling code is not included in this version of the patch so we will need to create a separate ticket for it.

Matias, thanks for your feedback! I included a small change to try to address the memory issue. We'll do tests on a large database to ensure that it is completely sorted. This will probably take the form of another ticket though.

Xuedi, this new patch will not address your issue. However, as the dialog now includes more user feedback (in particular a “Preparing pictures for import…” progress bar), it would be useful if you could confirm what is the position of the progress bar when the crash happens (start, middle, end?). I suspect that it chokes on one particular photo or tag entry in the database. In order to fix that, I need to make the error handling code more forgiving about errors so that it can recover from unexpected values better. As this is not trivial, it will probably be the subject of another ticket but it will be my priority to get it fixed before release.

Committed. Thanks Bruno for all the great work!

@brunogirin Great work, i just saw the patch made it into svn, the progress-bar and handling is perfect as well as the function, i could import all my database into shotwell !!!

Thanks a lot, karma up!

xuedi

Replying to [comment:49 xuedi]:

@brunogirin Great work, i just saw the patch made it into svn, the progress-bar and handling is perfect as well as the function, i could import all my database into shotwell !!!

Thanks a lot, karma up!

xuedi

Thanks for the feedback, I'm glad to see that it now works for you!