Would you expect the rotated photo to be a separate file or just a recording that the image has been rotated?

Not sure. If it is really 100% lossless, I guess just a recording is fine.

Well it's lossless since it doesn't modify the actual image data, it sets the exif orientation.  But does it modify the exif data in the image itself, or only in the photos.db?

This suggests to me that the modification versioning code should be extended to store exif-only modifications as binary diff-type files.  Perhaps this should be filed as a separate bug however.

(In reply to comment #3)
> But does it modify the exif data in the image itself, or
> only in the photos.db?

It changes the image file. There's nothing about orientation in photos.db

And I can confirm, rotating jpeg in f-spot is 100% lossless. Only headers are touched. Even the file size remains the same.

My view about tagging as modified event though it's lossless: creating a version under f-spot is creating a new file. Why creating a new file (using disk space) if the content IS the same...

(In reply to comment #4)
> My view about tagging as modified event though it's lossless: creating a
> version under f-spot is creating a new file. Why creating a new file (using
> disk space) if the content IS the same...

Because some of us are paranoid about touching the original photo data? ;)

I'm attracted to f-spot because there's generally a separation between the data pulled from the camera (by whatever method) and the modifications and annotations made by f-spot. I rest easy knowing that if I wanted to switch from using f-spot I can retreive all my original photos without any mangling - I can always roll back to step 0.

Rotation is inconsistent with this approach because the image file has been changed, even though the change is only in the header. rsync would think it a different file, for instance.

(Indeed, my ideal photo manager would take this separation further. I'd be even happier if I knew that f-spot opened originals read-only; and had the option for modified images to live in a separate directory structure to the originals, which could stay in the hierarchy they inherited from the camera)

Indeed, all you have to do to make f-spot work like your "ideal photo manager" is backup your photos first!  (Doesn't everybody do this?)  After I've imported my photos to f-spot, I move the files off my memory cards into a backup server.  Just makes sense to me...

And I think it would be nice if revisions for image rotations were stored as file diffs instead of a complete copy, so that you can easily rebuild the original file.

I think there are two issues here:

1) whether a rotated image should be a modified copy
2) whether a user imports with or without Copy; and how much control a user has over where, and in what heirarchy, f-spot stores photos

and that the second influences the first:

- a user who selects "Copy" on import expects f-spot to have total control of the photos its managing, and won't be surprised when f-spot modifies an image (e.g. rotation).

- however, a user who imports _without_ copy starts with an expectation of separation between the logical management of photos (f-spot) and the storage of the original files on disk. From this perspective, any modification of the source file would be surprising.

(In reply to comment #6)
> Indeed, all you have to do to make f-spot work like your "ideal photo manager"
> is backup your photos first!  (Doesn't everybody do this?)  After I've imported
> my photos to f-spot, I move the files off my memory cards into a backup server.

Don't get me wrong, I think f-spot is excellent. And yes, I keep an archive/backup copy of the photos as they arrive from the camera. I then import into f-spot - but I care about any modifications made therein, so I also backup the f-spot Photos directory. I know disk space is cheap, but I'm making alot of copies/backups of photos which are, in the most part, the same.

I won't take sides on how f-spot should handle image originals, but I would like to hack implement actual rotation of the image (instead of a change to EXIF data). I noticed in RotateCommand.cs there is an additional method called RotateCoefficients which seems to make a "physical" rotation of the image through JpegUtils. I changed
Rotate (original_path, direction);
in the step method of RotateOperation to:
RotateCoefficients (original_path, direction);
to test things. Unfortunately when the rotate button is pressed, the .tmp image is created and f-spot explodes explaining:
f-spot: symbol lookup error: /opt/gnome/lib/f-spot/libfspotjpegtran.so.0: undefined symbol: exif_data_new_from_data

Drilling down through the code, /f-spot/libjpegtran/jpeg-data.c is the source of the call to method exif_data_new_from_data which is defined in /f-spot/src/Exif.cs .

Any ideas about how to resolve this issue or implement "physical" rotation through other means. I'll continue to look into it, but you mono people change languages too many times and I have trouble telling where libraries connect.

Furthermore, if rotation is achieved, how would one do it through versioning. I haven't yet read through all the version and imagequery calls necessary to use such a method.

Colin

Colin (comment #8),
RotateCoefficient works fine, but is not used in f-spot. Try to clean and re-install f-spot to fix your particular issue.
Real rotation (using coefficient) is done now using the OrientationFilter when exporting images.

(In reply to comment #9)
> Colin (comment #8),
> RotateCoefficient works fine, but is not used in f-spot. Try to clean and
> re-install f-spot to fix your particular issue.
> Real rotation (using coefficient) is done now using the OrientationFilter when
> exporting images.
> 

Thanks, your absolutely right, it does work. I'm using arch linux and the AUR (arch user repository) has an ABS (arch build system, script based compiling and package building) which I was using, yet for some reason this system shows my experienced problem no matter the amount of package cleaning and upgrading I do. Since it works well manually from cvs (btw, abs builds from cvs), I'll probably be too lazy to hack versioning into the system. But if anyone provides a rotation versioning solution or patch, I'd love to hear it.

Created attachment 101498 [details] [review]
create-new-version-on-rotate.patch

with this patch, F-spot creates a new photo version on rotate IF a gconf key is set. 

the patch includes also the one at bug 505125. I know that this sucks but it wouldn't have made sense without it IMHO.

Created attachment 101514 [details] [review]
create-new-version-on-rotate.patch

I've corrected a few bugs, thanks to Nicke testing.

(In reply to comment #12)
> Created an attachment (id=101514) [edit]
> create-new-version-on-rotate.patch
> 
> I've corrected a few bugs, thanks to Nicke testing.
> 

I have successfully applied this to the Ubuntu 7.10 package (F-Spot 0.4.0), and it works for me. I hope this will be included in some form ;). Thanks!

no need for a new gconf key, use the 'write metadata' one

Created attachment 103011 [details] [review]
create-new-version-on-rotate.patch

Updated as per comment #14.

This is just a vote in the negative. I think the current implementation makes lots of sense. Why would you need another copy of a photo that is exactly the same except for EXIF information? Plus, why would you want to revert a photo to a sideways orientation? Plus, if you did want to do that, why not alter the EXIF information back to the original, rather than making two copies? 

I see the value in the system of modifying originals when the change is lossy, but with lossless changes, I just don't see why we shouldn't just make the change....

I completely agree with not making another copy of the image and wasting disk space.  I think ideally, there would be a way to mark it as modified and only store a diff from the original file, that way if someone wanted to revert it, for whatever reason, it can simply be patched back.  The biggest scenario I can see for this, is when you are trying to compare f-spot photos with photos archives from another source, comparing md5's or something.  It just seems to "make sense" that if any kind of modification is made, it should be marked as such.

Comment on attachment 103011 [details] [review]
create-new-version-on-rotate.patch

Maintenance update: In the past we've been less than stellar in reviewing patches. As such we have a pile of patches in bugzilla which are outdated and don't apply anymore. Am currently marking all of these as "needs-work". My apologies for this.

Since I've become a maintainer of the project, I've set the personal rule of quickly reviewing all patches, to avoid that this happens again. If you (or anyone) wants to go through the trouble of updating this patch, please talk to us to figure out if it fits in the F-Spot long term roadmap.

Should you, in the future, notice a patch lingering around for too long, please notify us immediately and we'll look into it, to avoid situations like these from happening again.

You can filter these mails by searching for ###F-OLDPATCHCLEANUP###

F-Spot has moved to https://github.com/f-spot/f-spot/issues

If this Bugzilla ticket is still valid in a recent version of F-Spot, please feel free to post this topic as a ticket in the F-Spot project on GitHub.

Closing this report as WONTFIX as part of Bugzilla Housekeeping as we are planning to shut down GNOME Bugzilla in favor of GNOME Gitlab.