GNOME Bugzilla – Bug 624794
Can't show neither thumbnail nor image if xmp file's XML is empty (0 Bytes)
Last modified: 2010-08-07 15:00:45 UTC
I would expect that F-Spot is not able to read the metadata then, but only because there is an FILENAME.xmp file (maybe modified/created by another application) which F-Spot can't read, F-Spot still should be able to show the image. The thumbnail works only with a JPG, the RAW thumbnails did not show up. This was the debug output: [4 Debug 08:13:40.818] Failed loading image for thumbnailing: file:///home/paul/Photos/2010/07/17/DSC_6302.NEF [4 Warn 08:13:40.818] Caught an exception - System.Xml.XmlException: Document element did not appear. Line 1, position 1. (in `System.Xml') at Mono.Xml2.XmlTextReader.Read () [0x00000] at System.Xml.XmlTextReader.Read () [0x00000] at System.Xml.XmlDocument.ReadNodeCore (System.Xml.XmlReader reader) [0x00000] at System.Xml.XmlDocument.ReadNode (System.Xml.XmlReader reader) [0x00000] at System.Xml.XmlDocument.Load (System.Xml.XmlReader xmlReader) [0x00000] at System.Xml.XmlDocument.LoadXml (System.String xml) [0x00000] at TagLib.Xmp.XmpTag..ctor (System.String data) [0x00011] in /usr/src/f-spot/lib/TagLib/TagLib/src/TagLib/Xmp/XmpTag.cs:256 at FSpot.Utils.SidecarXmpExtensions.ParseXmpSidecar (TagLib.Image.File file, IFileAbstraction resource) [0x00039] in /usr/src/f-spot/src/Utils/SidecarXmpExtensions.cs:25 at FSpot.Utils.Metadata.Parse (Hyena.SafeUri uri) [0x00074] in /usr/src/f-spot/src/Utils/Metadata.cs:26 at FSpot.Imaging.BaseImageFile..ctor (Hyena.SafeUri uri) [0x00014] in /usr/src/f-spot/src/Imaging/ImageFile.cs:188 at FSpot.Imaging.Tiff.NefFile..ctor (Hyena.SafeUri uri) [0x00000] in /usr/src/f-spot/src/BitConverter.cs:1 at (wrapper managed-to-native) System.Reflection.MonoCMethod:InternalInvoke (object,object[],System.Exception&) at System.Reflection.MonoCMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. (in `mscorlib') at System.Reflection.MonoCMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] at System.Reflection.MonoCMethod.Invoke (BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] at System.Activator.CreateInstance (System.Type type, BindingFlags bindingAttr, System.Reflection.Binder binder, System.Object[] args, System.Globalization.CultureInfo culture, System.Object[] activationAttributes) [0x00000] at System.Activator.CreateInstance (System.Type type, System.Object[] args, System.Object[] activationAttributes) [0x00000] at System.Activator.CreateInstance (System.Type type, System.Object[] args) [0x00000] at FSpot.Imaging.ImageFile.Create (Hyena.SafeUri uri) [0x0001e] in /usr/src/f-spot/src/Imaging/ImageFile.cs:128
Created attachment 166195 [details] [review] Make Metadata and Sidecar file parsing more reliable
Created attachment 166196 [details] [review] Make Metadata and Sidecar file parsing more reliable
There are unit tests for the sidecars. Could you add test cases with these failure conditions?
Created attachment 166374 [details] [review] Make Metadata and Sidecar file parsing more reliable The application should not crash when reading metadata or sidecar files fail.
Created attachment 166375 [details] [review] Add tests for reading broken sidecar fiels and broken metadata The tests should ensure that broken sidecar files or broken image files do cause metadata reading to fail. The new tests in MetadataTest.cs are very similar to the ones in SidecarXmpExtensionsTests.cs, but they use different utility methods, which also should be tested.
Created attachment 166376 [details] [review] Make Metadata and Sidecar file parsing more reliable The application should not crash when reading metadata or sidecar files fail.
Created attachment 166377 [details] [review] Add tests for reading broken sidecar fiels and broken metadata The tests should ensure that broken sidecar files or broken image files do cause metadata reading to fail. The new tests in MetadataTest.cs are very similar to the ones in SidecarXmpExtensionsTests.cs, but they use different utility methods, which also should be tested.
Comment on attachment 166377 [details] [review] Add tests for reading broken sidecar fiels and broken metadata Test files missing.
Review of attachment 166376 [details] [review]: Question: ::: src/Utils/SidecarXmpExtensions.cs @@ +14,3 @@ /// tag of file by the parsed data. /// </summary> + public static bool ParseXmpSidecar (this TagLib.Image.File file, TagLib.File.IFileAbstraction resource) Are the return values used anywhere?
Created attachment 166382 [details] [review] Make Metadata and Sidecar file parsing more reliable The application should not crash when reading metadata or sidecar files fail.
Created attachment 166383 [details] [review] Add tests for reading broken sidecar fiels and broken metadata The tests should ensure that broken sidecar files or broken image files do cause metadata reading to fail. The new tests in MetadataTest.cs are very similar to the ones in SidecarXmpExtensionsTests.cs, but they use different utility methods, which also should be tested.
Attachment 166382 [details] pushed as fac7e48 - Make Metadata and Sidecar file parsing more reliable Attachment 166383 [details] pushed as 9518bea - Add tests for reading broken sidecar fiels and broken metadata Committed these patches. Would like to have the output silenced during tests though, so I'm not closing the bug yet. Would that be possible?
Created attachment 167323 [details] [review] Silence debug output.
Attachment 167323 [details] pushed as 4d9ef47 - Silence debug output. Silenced the debug output. Please make an effort to maintain what you submit Mike. I know this ain't exactly the most fun thing to do, but it's even less fun if I have to do all of it.