Skip to content
This repository has been archived by the owner on Jun 23, 2023. It is now read-only.

Test case for #77 #87

Merged
merged 10 commits into from
Feb 3, 2017
Merged

Test case for #77 #87

merged 10 commits into from
Feb 3, 2017

Conversation

svenbluege
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) to 67.225% when pulling 113d922 on svenbluege:master into b044158 on lsolesen:master.

1 similar comment
@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage increased (+1.5%) to 67.225% when pulling 113d922 on svenbluege:master into b044158 on lsolesen:master.

@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage increased (+1.5%) to 67.225% when pulling 113d922 on svenbluege:master into b044158 on lsolesen:master.

@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage increased (+1.5%) to 67.225% when pulling 9547b7d on svenbluege:master into 7dadd2a on lsolesen:master.

@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage increased (+1.6%) to 67.281% when pulling 392d5d6 on svenbluege:master into 511b159 on lsolesen:master.

@weberhofer
Copy link
Collaborator

I have updated PEL not to fail on a position where it throws an exception. But it's really hard to debug the issue itself.
When somebody finds a solution we should merge the test into PEL.

@mondrake
Copy link
Contributor

mondrake commented Feb 3, 2017

The issue is, I think, more general as it affects any tag that is represented by a string, For example 'Make', 'ImageDescription', 'Software', 'Artist' all have the last character trimmed. See dump below

Tag Text Value
ifd: 0
ImageDescription Manhattan Skyline from Brooklyn Bridge Park Greenwa 'Manhattan Skyline from Brooklyn Bridge Park Greenwa'
Make Cano 'Cano'
Model Canon EOS 5D Mark II 'Canon EOS 5D Mark II'
XResolution 72/1 array ( 0 => 72, 1 => 1, )
YResolution 72/1 array ( 0 => 72, 1 => 1, )
ResolutionUnit Inch 2
Software Adobe Photoshop Lightroom 6.7 (Windows 'Adobe Photoshop Lightroom 6.7 (Windows'
DateTime 2016:11:25 12:48:12 1480078092.0
Artist Phil Steels Photograph 'Phil Steels Photograph'
Copyright Copyright 2016 (Photographer) array ( 0 => 'Copyright 2016', 1 => '', )
ifd: 1
Compression JPEG compression 6
XResolution 72/1 array ( 0 => 72, 1 => 1, )
YResolution 72/1 array ( 0 => 72, 1 => 1, )
ResolutionUnit Inch 2
ifd: Exif
ExposureTime 180 sec. array ( 0 => 180, 1 => 1, )
FNumber f/22.0 array ( 0 => 22, 1 => 1, )
ExposureProgram 9 9
ISOSpeedRatings 50 50
ExifVersion Exif Version 2.3 2.29999999999999982236431605997495353221893310546875
DateTimeOriginal 2016:11:25 12:48:12 1480078092.0
DateTimeDigitized 2016:11:25 12:48:12 1480078092.0
ShutterSpeedValue -7491853/1000000 sec. (APEX: 0) array ( 0 => -7491853, 1 => 1000000, )
ApertureValue f/22.0 array ( 0 => 8918863, 1 => 1000000, )
ExposureBiasValue 0.0 array ( 0 => 0, 1 => 1, )
MaxApertureValue 4/1 array ( 0 => 4, 1 => 1, )
MeteringMode Center-Weighted Average 2
Flash Flash did not fire, compulsory flash mode. 16
FocalLength 24.0 mm array ( 0 => 24, 1 => 1, )
SubSecTimeOriginal 0 '0'
ColorSpace sRGB 1
FocalPlaneXResolution 382423/97 array ( 0 => 382423, 1 => 97, )
FocalPlaneYResolution 320000/81 array ( 0 => 320000, 1 => 81, )
FocalPlaneResolutionUnit Inch 2
CustomRendered Normal process 0
ExposureMode Auto exposure 0
WhiteBalance Auto white balance 0
SceneCaptureType Standard 0
ifd: GPS
ifd: Interoperability

@weberhofer
Copy link
Collaborator

You are right. I have had a look at it some weeks ago. Unfortunately the parsing process is really complex and I could not find the problem. Currently I don't have time to debug that issue...

@svenbluege
Copy link
Contributor Author

My main question is: is the image broken? Other tools seam to be able to read the information properly. I did some debugging but I ended up with no clue. At least (think) I saw that the length of content is specified one character lower than what is the expected content length.

@weberhofer
Copy link
Collaborator

weberhofer commented Feb 3, 2017

Some generators could place a null terminator at the end of the string while others don't. Maybe that lenght-1 at the end should first check the last character?
I didn't check that but it sounds reasonable.

@mondrake
Copy link
Contributor

mondrake commented Feb 3, 2017

I was coming to same conclusion. In this test image there's no null terminator. Patch soon.

@mondrake
Copy link
Contributor

mondrake commented Feb 3, 2017

I made a PR, but probably to the wrong repo, svenbluege#1

Sorry I am not familiar with github PR process.

The fix seems to be pretty easy, always take full size of the tag value for PelEntryAscii.

Can anybody replicate the proposed change to this repo and see if tests pass?

@mondrake
Copy link
Contributor

mondrake commented Feb 3, 2017

Dump (partial, only ifd0) with patch applied:

Tag Text Value
ifd: 0
ImageDescription Manhattan Skyline from Brooklyn Bridge Park Greenway 'Manhattan Skyline from Brooklyn Bridge Park Greenway'
Make Canon 'Canon'
Model Canon EOS 5D Mark III 'Canon EOS 5D Mark III'
XResolution 72/1 array ( 0 => 72, 1 => 1, )
YResolution 72/1 array ( 0 => 72, 1 => 1, )
ResolutionUnit Inch 2
Software Adobe Photoshop Lightroom 6.7 (Windows) 'Adobe Photoshop Lightroom 6.7 (Windows)'
DateTime 2016:11:25 12:48:12 1480078092.0
Artist Phil Steels Photography 'Phil Steels Photography'
Copyright Copyright 2016 (Photographer) array ( 0 => 'Copyright 2016', 1 => '', )

@coveralls
Copy link

coveralls commented Feb 3, 2017

Coverage Status

Coverage increased (+1.6%) to 67.281% when pulling c83792e on svenbluege:master into 958d94e on lsolesen:master.

@coveralls
Copy link

coveralls commented Feb 3, 2017

Coverage Status

Coverage increased (+1.6%) to 67.281% when pulling 9a91732 on svenbluege:master into c19abee on lsolesen:master.

@weberhofer
Copy link
Collaborator

Thanks a lot for the test - I'll merge #77 is solved.

@weberhofer weberhofer closed this Feb 3, 2017
@weberhofer weberhofer reopened this Feb 3, 2017
@coveralls
Copy link

coveralls commented Feb 3, 2017

Coverage Status

Coverage increased (+1.6%) to 67.281% when pulling 9a91732 on svenbluege:master into c19abee on lsolesen:master.

@weberhofer weberhofer merged commit c9e3919 into pel:master Feb 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants