-
Notifications
You must be signed in to change notification settings - Fork 278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix 1343 png 0.27 #1351
Fix 1343 png 0.27 #1351
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -304,6 +304,7 @@ namespace Exiv2 { | |
bool zTXt = std::strcmp(chType,"zTXt")== 0; | ||
bool iCCP = std::strcmp(chType,"iCCP")== 0; | ||
bool iTXt = std::strcmp(chType,"iTXt")== 0; | ||
bool eXIf = std::strcmp(chType,"eXIf")== 0; | ||
|
||
// for XMP, ICC etc: read and format data | ||
bool bXMP = option == kpsXMP && findi(dataString,xmpKey)==0; | ||
|
@@ -313,7 +314,7 @@ namespace Exiv2 { | |
bool bSoft = option == kpsRecursive && findi(dataString,softKey)==0; | ||
bool bComm = option == kpsRecursive && findi(dataString,commKey)==0; | ||
bool bDesc = option == kpsRecursive && findi(dataString,descKey)==0; | ||
bool bDump = bXMP || bICC || bExif || bIptc || bSoft || bComm || bDesc ; | ||
bool bDump = bXMP || bICC || bExif || bIptc || bSoft || bComm || bDesc || eXIf ; | ||
|
||
if( bDump ) { | ||
DataBuf dataBuf; | ||
|
@@ -339,6 +340,9 @@ namespace Exiv2 { | |
if ( iTXt ) { | ||
bGood = (start+3) < dataOffset ; // good if not a nul chunk | ||
} | ||
if ( eXIf ) { | ||
bGood = true ;// eXIf requires no pre-processing) | ||
} | ||
|
||
// format is content dependent | ||
if ( bGood ) { | ||
|
@@ -383,6 +387,11 @@ namespace Exiv2 { | |
out.write((const char*)decoded.pData_,decoded.size_); | ||
bLF = true; | ||
} | ||
if ( eXIf && option == kpsRecursive ) { | ||
// create memio object with the data, then print the structure | ||
BasicIo::AutoPtr p = BasicIo::AutoPtr(new MemIo(data,dataOffset)); | ||
printTiffStructure(*p,out,option,depth); | ||
} | ||
|
||
if ( bLF ) out << std::endl; | ||
} | ||
|
@@ -448,8 +457,11 @@ namespace Exiv2 { | |
|
||
/// \todo analyse remaining chunks of the standard | ||
// Perform a chunk triage for item that we need. | ||
if (chunkType == "IEND" || chunkType == "IHDR" || chunkType == "tEXt" || chunkType == "zTXt" || | ||
chunkType == "iTXt" || chunkType == "iCCP") { | ||
if(chunkType == "IEND" || chunkType == "IHDR" | ||
|| chunkType == "tEXt" || chunkType == "zTXt" | ||
|| chunkType == "eXIf" | ||
|| chunkType == "iTXt" || chunkType == "iCCP" | ||
){ | ||
DataBuf chunkData(chunkLength); | ||
readChunk(chunkData, *io_); // Extract chunk data. | ||
|
||
|
@@ -463,6 +475,13 @@ namespace Exiv2 { | |
PngChunk::decodeTXTChunk(this, chunkData, PngChunk::zTXt_Chunk); | ||
} else if (chunkType == "iTXt") { | ||
PngChunk::decodeTXTChunk(this, chunkData, PngChunk::iTXt_Chunk); | ||
} else if (chunkType == "eXIf") { | ||
ByteOrder bo = TiffParser::decode(exifData(), | ||
iptcData(), | ||
xmpData(), | ||
chunkData.pData_, | ||
chunkData.size_); | ||
setByteOrder(bo); | ||
} else if (chunkType == "iCCP") { | ||
// The ICC profile name can vary from 1-79 characters. | ||
uint32_t iccOffset = 0; | ||
|
@@ -533,16 +552,17 @@ namespace Exiv2 { | |
// Write PNG Signature. | ||
if (outIo.write(pngSignature, 8) != 8) throw Error(kerImageWriteFailed); | ||
|
||
DataBuf cheaderBuf(8); // Chunk header : 4 bytes (data size) + 4 bytes (chunk type). | ||
DataBuf cheaderBuf(9); // Chunk header : 4 bytes (data size) + 4 bytes (chunk type) + nul | ||
cheaderBuf.pData_[8] = 0; | ||
|
||
while(!io_->eof()) | ||
{ | ||
// Read chunk header. | ||
|
||
std::memset(cheaderBuf.pData_, 0x00, cheaderBuf.size_); | ||
long bufRead = io_->read(cheaderBuf.pData_, cheaderBuf.size_); | ||
long bufRead = io_->read(cheaderBuf.pData_, 8); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case we additional byte is not needed at the end, I would revert this change and use |
||
if (io_->error()) throw Error(kerFailedToReadImageData); | ||
if (bufRead != cheaderBuf.size_) throw Error(kerInputDataReadFailed); | ||
if (bufRead != 8) throw Error(kerInputDataReadFailed); | ||
|
||
// Decode chunk data length. | ||
|
||
|
@@ -551,7 +571,7 @@ namespace Exiv2 { | |
|
||
// Read whole chunk : Chunk header + Chunk data (not fixed size - can be null) + CRC (4 bytes). | ||
|
||
DataBuf chunkBuf(8 + dataOffset + 4); // Chunk header (8 bytes) + Chunk data + CRC (4 bytes). | ||
DataBuf chunkBuf(8 + dataOffset + 4); // Chunk header (8 bytes) + Chunk data + CRC (4 bytes). | ||
memcpy(chunkBuf.pData_, cheaderBuf.pData_, 8); // Copy header. | ||
bufRead = io_->read(chunkBuf.pData_ + 8, dataOffset + 4); // Extract chunk data + CRC | ||
if (io_->error()) throw Error(kerFailedToReadImageData); | ||
|
@@ -561,16 +581,21 @@ namespace Exiv2 { | |
memcpy(szChunk,cheaderBuf.pData_ + 4,4); | ||
szChunk[4] = 0; | ||
|
||
if (!memcmp(cheaderBuf.pData_ + 4, "IEND", 4)) | ||
if ( !strcmp(szChunk,"IEND") ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 for replacing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you like me to revert some of this stuff and update the PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nono, the comment was positive, I think it is more readable to use |
||
{ | ||
// Last chunk found: we write it and done. | ||
#ifdef EXIV2_DEBUG_MESSAGES | ||
std::cout << "Exiv2::PngImage::doWriteMetadata: Write IEND chunk (length: " << dataOffset << ")\n"; | ||
#endif | ||
if (outIo.write(chunkBuf.pData_, chunkBuf.size_) != chunkBuf.size_) throw Error(kerImageWriteFailed); | ||
if (outIo.write(chunkBuf.pData_, chunkBuf.size_) != chunkBuf.size_) | ||
throw Error(kerImageWriteFailed); | ||
return; | ||
} | ||
else if (!memcmp(cheaderBuf.pData_ + 4, "IHDR", 4)) | ||
else if ( !strcmp(szChunk, "eXIf") ) { | ||
; // do nothing Exif metdata is written following IHDR | ||
; // as zTXt chunk with signature Raw profile type exif__ | ||
} | ||
else if ( !strcmp(szChunk, "IHDR") ) | ||
{ | ||
#ifdef EXIV2_DEBUG_MESSAGES | ||
std::cout << "Exiv2::PngImage::doWriteMetadata: Write IHDR chunk (length: " << dataOffset << ")\n"; | ||
|
@@ -672,10 +697,10 @@ namespace Exiv2 { | |
} | ||
} | ||
} | ||
else if (!memcmp(cheaderBuf.pData_ + 4, "tEXt", 4) || | ||
!memcmp(cheaderBuf.pData_ + 4, "zTXt", 4) || | ||
!memcmp(cheaderBuf.pData_ + 4, "iTXt", 4) || | ||
!memcmp(cheaderBuf.pData_ + 4, "iCCP", 4)) | ||
else if (!strcmp(szChunk, "tEXt") || | ||
!strcmp(szChunk, "zTXt") || | ||
!strcmp(szChunk, "iTXt") || | ||
!strcmp(szChunk, "iCCP")) | ||
{ | ||
DataBuf key = PngChunk::keyTXTChunk(chunkBuf, true); | ||
if (compare("Raw profile type exif", key, 21) || | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
STRUCTURE OF PNG FILE: 1343_empty.png | ||
address | chunk | length | data | checksum | ||
8 | IHDR | 13 | .......d.... | 0x4ce4e85c | ||
33 | sRGB | 1 | | 0xaece1ce9 | ||
46 | gAMA | 4 | .... | 0x0bfc6105 | ||
62 | pHYs | 9 | ......... | 0xc76fa864 | ||
83 | IDAT | 257 | x^..1..0......t.....h......z.. | 0x64465429 | ||
352 | IEND | 0 | | 0xae426082 | ||
|
||
STRUCTURE OF PNG FILE: 1343_empty.png | ||
address | chunk | length | data | checksum | ||
8 | IHDR | 13 | .......d.... | 0x4ce4e85c | ||
33 | iTXt | 39 | Description.....x.K.H.KOMQH... | 0xe0a1c8ce | ||
changed comment | ||
84 | sRGB | 1 | | 0xaece1ce9 | ||
97 | gAMA | 4 | .... | 0x0bfc6105 | ||
113 | pHYs | 9 | ......... | 0xc76fa864 | ||
134 | IDAT | 257 | x^..1..0......t.....h......z.. | 0x64465429 | ||
403 | IEND | 0 | | 0xae426082 | ||
changed comment | ||
STRUCTURE OF PNG FILE: 1343_comment.png | ||
address | chunk | length | data | checksum | ||
8 | IHDR | 13 | .......d.... | 0x4ce4e85c | ||
33 | sRGB | 1 | | 0xaece1ce9 | ||
46 | gAMA | 4 | .... | 0x0bfc6105 | ||
62 | pHYs | 9 | ......... | 0xc76fa864 | ||
83 | IDAT | 257 | x^..1..0......t.....h......z.. | 0x64465429 | ||
352 | tEXt | 24 | Comment.Can you read me? | 0x83cad5fa | ||
388 | IEND | 0 | | 0xae426082 | ||
|
||
STRUCTURE OF PNG FILE: 1343_comment.png | ||
address | chunk | length | data | checksum | ||
8 | IHDR | 13 | .......d.... | 0x4ce4e85c | ||
33 | iTXt | 39 | Description.....x.K.H.KOMQH... | 0xe0a1c8ce | ||
changed comment | ||
84 | sRGB | 1 | | 0xaece1ce9 | ||
97 | gAMA | 4 | .... | 0x0bfc6105 | ||
113 | pHYs | 9 | ......... | 0xc76fa864 | ||
134 | IDAT | 257 | x^..1..0......t.....h......z.. | 0x64465429 | ||
403 | tEXt | 24 | Comment.Can you read me? | 0x83cad5fa | ||
439 | IEND | 0 | | 0xae426082 | ||
changed comment | ||
STRUCTURE OF PNG FILE: 1343_exif.png | ||
address | chunk | length | data | checksum | ||
8 | IHDR | 13 | .......d.... | 0x4ce4e85c | ||
33 | sRGB | 1 | | 0xaece1ce9 | ||
46 | gAMA | 4 | .... | 0x0bfc6105 | ||
62 | pHYs | 9 | ......... | 0xc76fa864 | ||
83 | IDAT | 257 | x^..1..0......t.....h......z.. | 0x64465429 | ||
352 | eXIf | 108 | MM.*.................J........ | 0x764e86e1 | ||
STRUCTURE OF TIFF FILE (MM): MemIo | ||
address | tag | type | count | offset | value | ||
10 | 0x010e ImageDescription | ASCII | 17 | 74 | Can you read me? | ||
22 | 0x011a XResolution | RATIONAL | 1 | 92 | 72/1 | ||
34 | 0x011b YResolution | RATIONAL | 1 | 100 | 72/1 | ||
46 | 0x0128 ResolutionUnit | SHORT | 1 | | 2 | ||
58 | 0x0213 YCbCrPositioning | SHORT | 1 | | 1 | ||
END MemIo | ||
472 | IEND | 0 | | 0xae426082 | ||
|
||
Exif.Image.ImageDescription Ascii 17 Can you read me? | ||
Exif.Image.XResolution Rational 1 72 | ||
Exif.Image.YResolution Rational 1 72 | ||
Exif.Image.ResolutionUnit Short 1 inch | ||
Exif.Image.YCbCrPositioning Short 1 Centered | ||
STRUCTURE OF PNG FILE: 1343_exif.png | ||
address | chunk | length | data | checksum | ||
8 | IHDR | 13 | .......d.... | 0x4ce4e85c | ||
33 | iTXt | 39 | Description.....x.K.H.KOMQH... | 0xe0a1c8ce | ||
changed comment | ||
84 | zTXt | 145 | Raw profile type exif..x.U.... | 0x2f6d89e8 | ||
STRUCTURE OF TIFF FILE (II): MemIo | ||
address | tag | type | count | offset | value | ||
10 | 0x010e ImageDescription | ASCII | 17 | 74 | Can you read me? | ||
22 | 0x011a XResolution | RATIONAL | 1 | 92 | 72/1 | ||
34 | 0x011b YResolution | RATIONAL | 1 | 100 | 72/1 | ||
46 | 0x0128 ResolutionUnit | SHORT | 1 | | 2 | ||
58 | 0x0213 YCbCrPositioning | SHORT | 1 | | 1 | ||
END MemIo | ||
241 | sRGB | 1 | | 0xaece1ce9 | ||
254 | gAMA | 4 | .... | 0x0bfc6105 | ||
270 | pHYs | 9 | ......... | 0xc76fa864 | ||
291 | IDAT | 257 | x^..1..0......t.....h......z.. | 0x64465429 | ||
560 | IEND | 0 | | 0xae426082 | ||
changed comment | ||
Exif.Image.ImageDescription Ascii 17 Can you read me? | ||
Exif.Image.XResolution Rational 1 72 | ||
Exif.Image.YResolution Rational 1 72 | ||
Exif.Image.ResolutionUnit Short 1 inch | ||
Exif.Image.YCbCrPositioning Short 1 Centered |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
#!/usr/bin/env bash | ||
# Test PNG support (-pS, -pc, -pa -c ) | ||
|
||
source ./functions.source | ||
|
||
( cd "$testdir" | ||
|
||
files="1343_empty.png 1343_comment.png 1343_exif.png" | ||
copyTestFiles $files | ||
for file in $files ; do | ||
for i in 1 2 ; do | ||
runTest exiv2 -pR $file | ||
runTest exiv2 -pc $file | ||
runTest exiv2 -pa $file | ||
runTest exiv2 -c 'changed comment' $file | ||
done | ||
done | ||
) > $results 2>&1 | ||
|
||
reportTest | ||
|
||
# That's all Folks! | ||
## |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand why we need to do this change.
You are changing the size from 8 to 9 but then the last byte is never used.
In line 556 you are setting byte[8] to 0, but that is already done in line 562 where we set the whole byte array to 0.
Could you explain why this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this to use strcmp() instead of memcmp() to inspect different chunks. You can revert that, if you prefer memcmp.()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, I think this change is not needed . We are passing
szChunk
to thestrcmp
function and notcheaderBuf
. You can still copy 4 bytes from cheaderBuf to szChunk and szChunk can have the 5th byte set to NULL.