Feature #1199
WebP file support
100%
Description
Created a patch to add support for the WebP file format. Created and tested with svn/0.25 using GIMP WebP plugin. https://github.com/draekko/gimp-webp (requires gimp 2.9.4+)
Files
Related issues
Associated revisions
#1199 Thank you to Ben for the patch. This corrects various matters and adds a first stab at WebPImage::printStructure()
#1199 WebPImage::printStructure() refactored to match implentation style of other image handlers.
#1199 Patch from Ben. http://dev.exiv2.org/issues/1199#note-37
#1199 Fixed exiv2 -pR/-pC/-pX Stonehenge2.webp to report exif/ICC/XMP
#1199 Fixed crash with iptc by disabling it, updated decodeChunk to do better header checks, added hexdump function.
#1199 (as was discussed) moved WebPImage::debugPrintHex to Internal::binaryToHex
#1199 removed warnings in stringToHex, enabled iptc support for WebPImage.
#1199 update to binaryToHex, should remove compile warnings and fix extra space padding.
#1199 Fixing MSVC build breaker involving long/uint64_t conflict. Minor changes to behaviour of Internal::binaryToHex()
#1199. Fixing msvc/64 build breakers (more size/long/uintXX_t conflicts).
#1199 Adding webp tests (work in progress, more tests will be added).
#1199 fixed handling deleting data, and adding iptc when injecting
refs #1199: Fix iclude
#1199 Updated exiv2.1 to add WEBP to format list.
#1199 update .pot file to add webpimage.cpp
#1199 WebP support. Changes to test suite. See http://dev.exiv2.org/issues/1199#note-79 for discussion.
#1199 updated date in exiv2.1
#1199 removed convenience converstion for iptc to xmp, apps should handle it directly since webp doesnt support iptc.
#1199 removed exif header padding on metadata writes.
#1199 Fixing exiv2 -dC image-path to delete ICC profile.
#1199 fixed -de -dc options, add -iC option to insert icc profiles
#1199 Adding tests for -dC -de -dx
#1199. Adding tests for -iC. Fixed bug in -pC. Test code added for -ix and -ie, however I don't think the options are working correctly.
#1199 Test suite changes and associate fixes to the code.
#1199 Fixed issue with -ix, wasnt padding odd sized chunks for exif/xmp.
#1199 Fix WebP printStructure(), readMetadata() and doWriteMetadata() to handle payload padding byte. Extended webp-test to cover -iX, -ix and -ie
#1199 minor code update, updated comments, added constants for headers, bit definitions, and misc.
#1199 fixed typo from cut/paste in previous commit causing exif flag not to be set for VP8X chunk.
#1199 Spit'n'polish on the loops in printStructure(), metadataRead() and doWriteMetadata(). Added tests with even byte count for ICCP and XMP Chunks.
#1199 Fixing webp-test.sh. Use iXX to inject XMP into file without XMP>Exif/IPTC conversion
#1199 Fixing build-breaker when -DDEBUG is used.
History
Updated by Robin Mills over 5 years ago
- File Moongirl.jpg Moongirl.jpg added
- Tracker changed from Bug to Feature
- Category set to image format
- Status changed from New to Assigned
- Assignee set to Robin Mills
- Target version set to 0.28
Ben
I'm on vacation on a motor boat in Finland. I'll look at your patch next weekend when I get home.
WebP is scheduled to be added in v0.27. #1048. I downloaded the SDK from Google about 18 months ago. I think it's fairly easy to use that to add webp support to Exiv2.
I don't have time at the moment look at your patch. If this introduces a complex build dependency on GIMP, it will not be accepted.
Updated by Ben Touchette over 5 years ago
I wasn't in a rush :)
I didn't want to add a dependency on libwebp. It has no dependencies and is self contained. I only tested it with the plug-in to make sure it worked through gexiv2 as i was working on that for said plugin at the time and from the exiv2 on the commandline.
Have a good one.
Updated by Robin Mills over 5 years ago
Ah, this sounds good. I'll have a look at this at the weekend. Beautiful day in the Finnish Archipelago.
Updated by Ben Touchette over 5 years ago
Lucky, i need a year off from life just to recuperate.
As for the patch it does borrow a lot of code from the other file formats that were already in there so feel free to change anything that needs to be changed or that i got wrong. It's only important that it works and gets included eventually.
:)
Updated by Ben Touchette over 5 years ago
Wondering if you had the opportunity to look at the patch?
Updated by Gilles Caulier over 5 years ago
The patch sound well written (not tested).
I vote to integrate this patch for next release. code is not too much intrusive and easy to check. I would integrate quickly webp support in digiKam. This imply to have propers metadata R/W support...
Gilles Caulier
Updated by Robin Mills over 5 years ago
- % Done changed from 0 to 10
- Estimated time set to 15.00 h
I looked at the patch on Wednesday while I was in Finland. I like that you're using BasicIo to perform the I/O. In fact it's a prerequisite for any image handler to be consistent with our other image handlers. When I looked at Google's libwebp code, I concluded that it may have to be rewritten to work with BasicIo. BasicIo support is essential to exiv2 to provide support for http and other types of I/O.
To integrate your patch into libexiv2, I will have to add test files and build support for CMake/Visual Studio/Autotools. Exiv2 has two totally different file parsers image->readMetadata()
(which have been there forever) and image->printStructure()
which I wrote to debug image files. It's not essential, however desirable, to include image->printStructure()
in webpimage.cpp.
I'm running late on v0.26 already. I don't have a lot to do for v0.26 (about 50 hours), however I do a life with other matters which require my time. So Gilles is entitled to his vote and I think it is desirable to include this in Exiv2 v0.26. If Gilles would like to provide the build, test and printStructure() support, he can submit the patch.
However, I know that Gilles has a family and other stuff to occupy his time in August. If you or Gilles can't do this additional work, I don't want to promise to deal with this while I still have outstanding v0.26 work to finish.
Updated by Gilles Caulier over 5 years ago
For the build structure, Only Cmake is on my mind.
Do not ask me to support MSVC, it's a waste of time. For whole digiKam now (800.000 lines of code + all shared libs as Qt5 + KF5) we use MXE cross compiler which can provide shared/static 32/64 bits targets, including installer... If you look into MXE project, you will see that Exiv2 is already supported as well and work like a charm.
Advantages : Only one OS to compile : Linux. No need Windows + MSVC. No risk of virus. No need extra MSVC runtime... etc. To debug, all GDB based IDE work fine. The target can be linked with a MSVC client application.
But, it's another subject.
For the test code, Ben, we need Webp test images + some code to check basic operations to read/write data structures in Exif and XMP. Of course this code must check results to complete tests. You will find test code samples for other image formats in test subdir, mostly written in bash. In test/data, we host image samples. I'm sure that one script already check this kind of basic operation for other formats, so take a look and add webp support.
If this test stage pass with webp, well this will be a good point to include your patch in next Exiv2 release.
Q : how did you complete some tests currently to validate your patch ?
Gilles Caulier
Updated by Ben Touchette over 5 years ago
A bit more work than i thought, i'll see what i can do.
@Gilles I downloaded several images (with and without animation) and manually added the icc profile, exif and xmp data since they were all missing those. Then i ran them through the code (standalone exiv2 program and through the gimp plug-in i was working on). I'd have to create my own test images since those aren't licensed for that kind of redistribution.
@Robin whats the cut-off date (approximately) so i can figure out if i can possibly do it in time :)
Updated by Robin Mills over 5 years ago
- File Stonehenge.webp Stonehenge.webp added
- File Stonehenge.jpg Stonehenge.jpg added
I've submitted your patch r4355. I agree with Gilles, this looks well written.
I'm submitted the build change for CMake/Visual Studio/Autotools. I've only tested autotools on MacOS-X. I'll inspect the buildserver log files later.
It's essential to support Visual Studio. DigiKam lives in the CMake world, however "native" Visual Studio in important to other users of Exiv2.
Currently Exiv2 supports Exif, IPTC and XMP metadata. Support for ICC profiles has been added in v0.26 (although not yet complete). I think suitable test images can be created with cwebp which ships with libwebp. This isn't a comprehensive test for your code, however it's an essential starting point. Here's the progess so far:
1061 rmills@rmillsmbp:~/temp $ curl -O http://clanmills.com/Stonehenge.jpg ; ls -alt Stonehenge.jpg % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 6599k 100 6599k 0 0 1298k 0 0:00:05 0:00:05 --:--:-- 1459k -rw-r--r--+ 1 rmills staff 6757827 8 Aug 15:52 Stonehenge.jpg 1062 rmills@rmillsmbp:~/temp $ cwebp Stonehenge.jpg -o Stonehenge.webp Saving file 'Stonehenge.webp' File: Stonehenge.jpg Dimension: 6000 x 4000 Output: 1936490 bytes Y-U-V-All-PSNR 38.52 45.95 46.86 39.94 dB block count: intra4: 61816 intra16: 31934 (-> 34.06%) skipped block: 15202 (16.22%) bytes used: header: 370 (0.0%) mode-partition: 302819 (15.6%) Residuals bytes |segment 1|segment 2|segment 3|segment 4| total macroblocks: | 5%| 21%| 22%| 50%| 93750 quantizer: | 36 | 34 | 29 | 19 | filter level: | 11 | 8 | 6 | 8 | 1063 rmills@rmillsmbp:~/temp $ od -a Stonehenge.webp | head -2 0000000 R I F F b 8c gs nul W E B P V P 8 sp 0000020 V 8c gs nul ? nl 94 9d soh * p etb ? si > 91 1064 rmills@rmillsmbp:~/temp $ exiv2 --verbose --version -g svn -g date -g time exiv2 0.25 001900 (64 bit build) date=Aug 8 2016 time=15:42:57 svn=4354 have_gmtime_r=1 have_timegm=1 xmlns=mediapro:http://ns.iview-multimedia.com/mediapro/1.0/ 1065 rmills@rmillsmbp:~/temp $ exiv2 -pa Stonehenge.webp 1 2 1066 rmills@rmillsmbp:~/temp $When we run
$ exiv2 -pa Stonehenge.webp
, it calls IsWebPType() and creates the WebPImage instance. WebPImage::readMetadata() and which calls WebPImage::decodeChunks() is called. And there the story ends. The metadata structures have not been created. I'm sure there's not much wrong here, however I don't have time at the moment to debug this.
The output 1 and 2 are coming from output prompts I have added to WebPImage::readMetadata() and WebPImage::decodeChunks(). I didn't intend to submit those lines - you should remove them of course.
I don't know that we have a "cut off" date. When v0.26 is feature complete, the code will be tested and tweaked for release by Andreas. I'm rather busy at the moment and hoped to have Exiv2 v0.26 ready by the end of June. Having missed the June date, I am caught up in other projects such as vacation and finishing the total remodel of our home.
Here are the next steps as I see it:
1) I'll support the build stuff. CMake/Visual Studio/Autotools will be working by 2016-08-09 12:00 UTC. The buildserver will be building and passing this by 2016-08-10 12:00 UTC.
2) Can you sync/build the trunk and debug WebPImage::decodeChunks()
Once we have the code working, we can discuss:
3) Test Harness and test files
4) Functionality (Exif, IPTC, XMP and ICC support)
5) Stuff we haven't thought of yet!
Updated by Ben Touchette over 5 years ago
Lol, it's just that the 50 hours comment made it sound imminent hence my asking if there was a deadline. As for the test, weird it worked here (famous development last words), must have omitted something by mistake when submitting. As for the rest sure, sounds like a deal. I'll get to it later today hopefully or tomorrow to see what is missing. For the ICC profiles i saw but couldn't get it working due to errors when testing it with gimp so put it aside for now and went to using the libwebp to add/remove it (for now) in the plugin and left it out for the patch.
Updated by Ben Touchette over 5 years ago
Also i wasn't sure about the IPTC stuff as webp doesn't support it so i have it try and copy it xmp. Hopefully thats the right way to handle that and since gimp has several bugs 2.9 witht i wasn't sure it was going into the webp file properly i need to recheck that as well with the command line app.
Updated by Gilles Caulier over 5 years ago
ExifTools do not show IPTC suppport in WebP.
But IPTC can be embeded in Exif. There is a standard tags for that. It's only recommend to do it with image format that support long chunk of byte array to host metadata. For ex, JPEG only support 64Kb for Exif, which is too small as IPTC can store 256Kb of preview image for ex.
So, this depend of WebP feature. If metadata chunk is small, forget IPTC in Exif. else you can do it in right exif tag.
But i recommend to do the same than ExifTools, to be homogeneous for the moment.
Gilles Caulier
Updated by Robin Mills over 5 years ago
Ben
Never underestimate what it takes to get code to work everywhere. When people say "it's done", they usually mean "it worked once on my computer". That's not "done". That is "just started". Getting it to work once on one test file is probably about 25% of the job (possibly less).
When folks (like Gilles) say "All Robin has to do is ....", notice the clever use of the words "Robin has to do it all.". The 50 hours left for v0.26 are documented here: http://dev.exiv2.org/news/2 under "Remaining work for Robin".
My wife is fixing dinner at the moment (I'm in England) and I've started to step through your code. There's a test in decodeChunks:
if (equalsWebPTag(chunkId, "VP8X")The file Stonehenge.webp has the chunkId == "VP8 ". I'll have to read the WebP spec to figure out what this is about.
Let's see if we can get this building/working/passing the tests on every platform, build system, 32/64 bit static/dynamic little-endian/big-endian without ether of us having to do too much work. We can do it. Let's do it!
Updated by Robin Mills over 5 years ago
I think I agree with Gilles position on IPTC. I usually agree with Gilles. If IPTC is encoded in a block in WebP, it would be good to respect it. Let's not worry about IPTC yet.
Here's the plan:
1) I'll support the build stuff. CMake/Visual Studio/Autotools will be working by 2016-08-09 12:00 UTC. The buildserver will be building and passing this by 2016-08-10 12:00 UTC.
2) Can you sync/build the trunk and debug WebPImage::decodeChunks()Once we have the code working, we can discuss:
3) Test Harness and test files
4) Functionality (Exif, IPTC, XMP and ICC support)
5) Stuff we haven't thought of yet!
I'm working on Step 1. Can you work on Step 2? We'll discuss IPTC in Step 4.
The biggest part of this project (like most projects) is probably Step 5 - dealing with stuff we don't know anything about yet. Of course, the battle cry of managers is "it shouldn't be like that", or the even more stupid "why didn't you tell me about this sooner". For example, we'll have to update the man page exiv2.1 and we may have to modify some sample applications. There may be hidden horrors in the webp specification. libwebp/src has 43000 lines of code and exiv2/src/webpimage.cpp has 430. It's possible that we are not 100x smarter than the Googlers who wrote libwebp.
Updated by Ben Touchette over 5 years ago
Robin i never said it was done lol, but i get what you mean. Software is never done, smh, i know i used to do this professionally in a previous life.
There are several VP8 (that being lossy and VP8L being lossless) type chunks, VP8X is the usually the first one (right after the header "RIFFxxxxWEBP" 12bytes long) describing image size and supported features. The wepb always sticks the 'ICCP' chunk right after that followed by the frame/layer/image data and ends with the rest of the metadata chunks. Odds are the stonehenge converted to webp doesn't contain copy of the metadata. I'll double check this but none of the images i found had any in them. Do a hex search for 'EXIF', 'XMP ', or 'ICCP' for the chunk headers in the file. If they're in there then its a bug if not the cwebp only copies the image data. I'll be trying it here in a bit as well. Btw 'VP8 ' should should be the data for a single image and 'ANIM' as well as 'ANMF' are the animation and frame data chunks. Webp has no IPTC chunk which was why i was asking. Time to do a quick check of all this here before heading of the Zzz land.
Here's the direct link to the container specs https://developers.google.com/speed/webp/docs/riff_container
Updated by Ben Touchette over 5 years ago
I opened up the stonehenge file after converting it with cwebp here and there is no metadata in it.
Updated by Ben Touchette over 5 years ago
- File Stonehenge.webp Stonehenge.webp added
@Robin: Here is the Stonehenge.webp i exported using my gimp plugin with the patch i submitted and it works fine here. Let me know how it works out for you.
Updated by Ben Touchette over 5 years ago
I can always add extra display messages for when no ICCP, XMP, and EXIF chunks are found.
Updated by Ben Touchette over 5 years ago
Last bit before heading off, you can use webpmux to add/remove metadata in a webp file which is what i used originally when i started tinkering.
Updated by Ben Touchette over 5 years ago
Doh, first bug already, the stonehenge file seems to not export well with my gimp plugin though all my previous files worked. Now to see if its the export of the exiv2 patch thats the problem. Will update back tomorrow.
Updated by Robin Mills over 5 years ago
- File Desk2016-08-09.jpg Desk2016-08-09.jpg added
Good News:
1 Stonehenge.webp does not have metadata
570 rmills@rmillsmbp:~/gnu/webp/libwebp $ exiv2 --verbose -pa ~/Stonehenge.webp File 1/1: /Users/rmills/Stonehenge.webp 1 2 /Users/rmills/Stonehenge.webp: No Exif data found in the file /Users/rmills/Stonehenge.webp: No IPTC data found in the file /Users/rmills/Stonehenge.webp: No XMP data found in the file 571 rmills@rmillsmbp:~/gnu/webp/libwebp $2 cwebp can preserve some metadata
577 rmills@rmillsmbp:~/gnu/webp/libwebp $ cwebp -metadata all ~/Stonehenge.jpg -o ~/Stonehenge.webp Saving file '/Users/rmills/Stonehenge.webp' File: /Users/rmills/Stonehenge.jpg Dimension: 6000 x 4000 Output: 1936490 bytes Y-U-V-All-PSNR 38.52 45.95 46.86 39.94 dB block count: intra4: 61816 intra16: 31934 (-> 34.06%) skipped block: 15202 (16.22%) bytes used: header: 370 (0.0%) mode-partition: 302819 (15.6%) Residuals bytes |segment 1|segment 2|segment 3|segment 4| total macroblocks: | 5%| 21%| 22%| 50%| 93750 quantizer: | 36 | 34 | 29 | 19 | filter level: | 11 | 8 | 6 | 8 | Metadata: * EXIF data: 15280 bytes * XMP data: 2579 bytes 578 rmills@rmillsmbp:~/gnu/webp/libwebp $ exiv2 -pa ~/Stonehenge.webp 1 2 Warning: Failed to decode Exif metadata. Exif.Image.DateTime Ascii 20 2015:07:16 20:25:28 Exif.Image.ImageDescription Ascii 13 Classic View Exif.Image.Rating Short 1 0 Iptc.Application2.RecordVersion Short 1 0 Iptc.Envelope.CharacterSet String 3 G Iptc.Application2.Caption String 12 Classic View Xmp.xmp.Rating XmpText 1 0 Xmp.xmp.ModifyDate XmpText 25 2015-07-16T20:25:28+01:00 Xmp.dc.description LangAlt 1 lang="x-default" Classic View 579 rmills@rmillsmbp:~/gnu/webp/libwebp $
3 CMake/Visual Studio/Autotool builds are working
4 I'll write WebPImage::printStructure()
I think this is a very valuable debugging tool. I hope to submit that code today.
5 What I'd like you to do
Perhaps we should have a discussion on Skype. I am 'clanmills'. Let's discuss your situation (location, availability, skills). We have to assemble some test files and write some test scripts. We can use the Stonehenge file, together with the libwebp utilities to generate copyright free images for the test suite. We should make the test images as small as possible to minimise the size of the svn source checkout.
We have to investigate readMetadata(). $ cwebp -metadata all
claimed to have written 12k bytes of Exif metadata, however there are only 3 keys being reported. There's a mysterious message Warning: Failed to decode Exif metadata
We are also reporting IPTC metadata. The XMP data looks correct.
580 rmills@rmillsmbp:~/gnu/webp/libwebp $ exiv2 -pa ~/Stonehenge.webp 1 2 Warning: Failed to decode Exif metadata. Exif.Image.DateTime Ascii 20 2015:07:16 20:25:28 Exif.Image.ImageDescription Ascii 13 Classic View Exif.Image.Rating Short 1 0 Iptc.Application2.RecordVersion Short 1 0 Iptc.Envelope.CharacterSet String 3 G Iptc.Application2.Caption String 12 Classic View Xmp.xmp.Rating XmpText 1 0 Xmp.xmp.ModifyDate XmpText 25 2015-07-16T20:25:28+01:00 Xmp.dc.description LangAlt 1 lang="x-default" Classic View 581 rmills@rmillsmbp:~/gnu/webp/libwebp $ exiv2 -pa --grep Xmp ~/Stonehenge.jpg Xmp.xmp.Rating XmpText 1 0 Xmp.xmp.ModifyDate XmpText 25 2015-07-16T20:25:28+01:00 Xmp.dc.description LangAlt 1 lang="x-default" Classic View 582 rmills@rmillsmbp:~/gnu/webp/libwebp $Investigating these odd things will be much easier when WebPImage::printStructure() has been implemented.
6 Perfect Weather in England
Beautiful day here and the forecast for the next week is perfect. Here's the view from my desk.
Updated by Robin Mills over 5 years ago
- File Stonehenge2.webp Stonehenge2.webp added
- % Done changed from 20 to 30
Updated by Gilles Caulier over 5 years ago
Ben,
About webp features that i need to implement in digiKam image loader, outside metadata, i'm right if i said :
- 8 and 16 bits colors depth.
- lossy and loss less compression.
- ARGB or RGB support.
- ICC color profile support.
Does lossless is only in 8 bits color depth or 16 bits is supported ?
RGBA color components are stored in image as PNG for ex ?
Other color spaces are supported, as YCrCb as JPEG ?
On wikipedia, information are not clear or perhaps not up to date. I don't yet take a look into libwebp, but as i see in your gimp plugin code, it's very easy to implement.
There is any way to have a progress info while load/decoding or encoding/save the image ?
Thanks in advance
Gilles Caulier
Updated by Ben Touchette over 5 years ago
@Gilles I haven't found enough information to properly answer you, so i'll try with what i know. As far as i can see its ARGB, ICC profiles are support (the gimp file plugin i'm working on exports it to them at any rate, lol) and lossy and lossless is support but for color depth it seems to be 8 bits only but i am not 100% sure on that part but the encoder seems to only output that. But the documentation is lacking in some areas so its hard to tell. It's RGBA only as far as i can tell, and how i implented it in the plugin.
@Robin Ha nice day, still early here on the other side of the pond :) and good to know about cwebp, honestly never used till last night lol.
I found two issues last night trying to code around them, turns out some height x width data should be in the VP8 and VP8L chunks as well and i need to reference that when no VP8X header is included to get h,w and second when no VP8X header is in the file i need to create one when adding either EXIF or XMP chunks or else the reference library (and related apps) give an error.
I never noticed it before because the exporter i was using was adding an ICC profile and was thus already creating the VP8X header. I'm poring over the webpmux code to see how i could implement that hopefully today or tomorrow worst case.
As for items 3 and 4 good to know, especially for item 3 i tried to edit the files but since i don't have those items to test with it was a shot in the dark :)
For item 5 as i said above i still have a few items to fix, once thats done i should be able to generate any combination of images as needed.
As for location i'm in Canada on the eastern side (EST time). I'll dust off my old Skype account i almost never use it now. Time for some breakfeast now.
Updated by Robin Mills over 5 years ago
Thanks for the update, Ben. Canada, eh? I'm an American Citizen, however I've only been in Canada at Niagara Falls. Would like to ride the train coast-to-coast one day. I'll try to get printStructure() implemented later today. Not only is that useful for debugging, I'll actually understand the WebP format when I have that done. So if you fix anything you know is broken, it will be helpful to fix it and send me a patch. After that we can tackle the test harness. If you get the files, I'll do the bash scripts.
Gilles: I can't answer your questions about ColorSpaces, depth and so on. The idea of having a progress callback for readMetadata() and writeMetadata() sounds good. I think that should be added to the Exiv2::Image class and be available for every type of Image. Can you raise a feature request on Redmine.
Updated by Ben Touchette over 5 years ago
@Robin Yep, I noticed that when i saw you were from San Jose, from your Skype profile :) Never been there but have driven out from here to Vancouver though half on the Canadian side and half on the American side (Detroit to Seattle). Yeah a train ride across would be nice though as i understand it there's a small stretch where where this isn't any service any longer somewhere in Ontario (though i may have misunderstood what i was told at the time or it may have changed since) so you'd have to switch off and on there.
Yeah working on adding the parts i noticed were missing like having to add reading the sizes properly for when the VP8X chunk is missing. Going to work on the writing part that needs fixing later on today or tomorrow morning.
Updated by Robin Mills over 5 years ago
We lived in San Jose (Silicon Valley) for 14 years and retired to our 'old' house in England 2 years ago. We've totally renovated our 40 year old house. All the construction work is finished and we're into decorating. We'll be pretty well done in September or October.
We've visited all 50 of the United States and crossed the States on all three trans-continental tracks. However, we have never been in Canada or Mexico. A deliberate choice. If you take on too much, you fail. We restricted ourselves to the States. We're both US Citizens and very happy to be 'home' in England. We met in High School in Scotland. So one of our 'bucket list' projects is to cross Canada by train. http://clanmills.com/BucketList.shtml
Right. I have to go with Alison to look at tiles for the kitchen, then I'll write WebPImage::printStructure(). I'm rather hoping that I can cut'n'paste your code for readMetadata() and get this done quickly. From your conversation with Gilles, I think you have a WebP parser here: https://github.com/draekko/gimp-webp
Updated by Ben Touchette over 5 years ago
@Gilles looks like was wrong about the colorspace see https://developers.google.com/speed/webp/docs/api and do a search in the page for colorspace. As far as i know right now all exporters are doing rgba.
Updated by Ben Touchette over 5 years ago
@Robin: hit a snag, seems like the encoder pads with extra \0 some chunks trying to figure it out and how to handle it in the writeMetadata function. Fixed the reading part to disregard those.
Updated by Ben Touchette over 5 years ago
- File exiv2-160810-3.diff exiv2-160810-3.diff added
@Robin New patch update which fixes all previous issues and adds printStructure. I did find one new issue where with a lossless image with alpha but without animation the height listed in the data is way too high (eg. should be 1333 and gets 17717). Trying to figure out where it gets mangled as all other variations work fine. This patch adds/changes to whats in the repo.
Updated by Ben Touchette over 5 years ago
Before i forget again, i get free errors with the gimp plugin when loading an image using whats in repo but when built from the stable version (0.25) it works fine.
Updated by Robin Mills over 5 years ago
- % Done changed from 30 to 40
Thanks for providing the patch. It's submitted r4360 Very good work indeed. Thank You very much.
1 Functionality
Here's what I see:
630 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS ~/Stonehenge2.webp STRUCTURE OF WEBP FILE: /Users/rmills/Stonehenge2.webp offset | chunk_type | length | data 0 | WEBP | 12 | RIFFH...WEBP 12 | VP8X | 10 | ....o..... 30 | VP8 | 1936470 | .....*p...>.D.I 1936508 | EXIF | 15280 | II*............ 1951796 | XMP | 2579 | <?xpacket begin 631 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa ~/Stonehenge2.webp Warning: Failed to decode Exif metadata. Exif.Image.DateTime Ascii 20 2015:07:16 20:25:28 Exif.Image.ImageDescription Ascii 13 Classic View Exif.Image.Rating Short 1 0 Iptc.Application2.RecordVersion Short 1 0 Iptc.Envelope.CharacterSet String 3 G Iptc.Application2.Caption String 12 Classic View Xmp.xmp.Rating XmpText 1 0 Xmp.xmp.ModifyDate XmpText 25 2015-07-16T20:25:28+01:00 Xmp.dc.description LangAlt 1 lang="x-default" Classic View 632 rmills@rmillsmbp:~/gnu/exiv2/trunk $The Exif data block is 15280 byte and that feels "about right" and it was 15288 in Stonehenge.jpg (which has a few additional bytes to identify it as Exif in at APP1 segment). However you're only reporting a few keys. I'm wondering if your call to copyXmpToExif() is clobbering something. Also, you have a warning Warning: Failed to decode Exif metadata. What's that about?
632 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS ~/Stonehenge.jpg STRUCTURE OF JPEG FILE: /Users/rmills/Stonehenge.jpg address | marker | length | data 2 | 0xd8 SOI | 0 4 | 0xe1 APP1 | 15288 | Exif..II*...................... 15294 | 0xe1 APP1 | 2610 | http://ns.adobe.com/xap/1.0/.<?x 17906 | 0xed APP13 | 96 | Photoshop 3.0.8BIM.......'..... 18004 | 0xe2 APP2 | 4094 | MPF.II*...............0100..... 22100 | 0xdb DQT | 132 22234 | 0xc0 SOF0 | 17 22253 | 0xc4 DHT | 418 22673 | 0xda SOS | 12 633 rmills@rmillsmbp:~/gnu/exiv2/trunk $
2 printStructure()
I was bored this afternoon and did some work on printStructure(). You have the right idea about what's to be done and you've implemented the -pS capability.
Some constructive comments about printStructure()
a) You've used sprintf to format strings. I have Internal:: functions to perform that
b) There are a load of Clang warnings about unsigned/signed comparisons
c) Deal with recursion to dump the Exif data (which is a TIFF formatted structure)
d) We should also implement the options used by
i) -pR/C/X (print Recursive, ICC Profile, XMP)
ii) -dC/E/I/X (delete ICC profile, Exif, IPTC and XMP)
So, to help you focus on the WebP matters, I'll take responsibility for printStructure() and develop it. I'll submit what I have so far which is functionally the same as you have. I've implemented -pS and just started to work on -pR.
3 Leaking memory
I'm not aware of that. Can you open a new issue in Redmine for that to ensure it is not forgotten.
4 Last night's build/test was totally successful.
Clang, GCC, Visual Studio 2005/8/10/12/13/15 (x86 & x64) was 100% successful and includes webpimage.cpp/.hpp
5 We can probably start working on the test suite on Friday
When we resolve the difference between the Exif metadata appearing from Stonehenge2.webp and Stonehenge.jpg we'll be ready to start working on the test suite. And while you're figuring out the mysteriously missing metadata, I'll complete WebPImage::printStructure().
6 Other matters
I looked at your contribution to Gimp (themes and back porting them). And you've been active on other projects. I encourage you to spend a little time on OpenHub to be sure your other projects are monitored and that your "Kudos" rating reflects your contributions. You may need to register projects such as the themes and plugin with OpenHub. It's painless and free - just a little of your time.
I'll email Andreas about granting you write access to the repos. Can't guess or promise when (or even if) he will do that.
Thanks very much for doing this work. It will be appreciated by many. Don't expect anybody to thank you - however that doesn't mean that it isn't appreciated! As a matter of fact, I really only hear from folks about bugs and issues. Being thanked is very unusual. However, I do appreciate and say Thank You, Ben for getting involved with this.
Updated by Ben Touchette over 5 years ago
1- If you get a chance maybe double check where i create the exif blob to see if did it right i may have missed something obvious. As for the actual data i was exporting from Gimp and it rewrites some data on export which i pass on to gexiv2 which gives it to exiv2 lib so its possible its also from there. As for the error message i'm not getting that on my end, all of them list fine. How was the file created?
2- Sounds good to me. You know better what you want there, i only did that to get things going and it helped me as well with those offsets because google doesn't follow their spec and pads any data payload with \0 on odd sized data chunks.
3- Not sure either but i think its the icc code which is ifdef'ed for the SVN version only. Not really high on my list just yet.
4- Sweet, good news.
5- Ok
6- Thanks for saying, and yeah i doesn't happen often enough considering the hours we pour into these apps. I will add a bit more on openhub once i figure it out and am done with this :) As for the repo well whenever it happens is good enough :)
Updated by Robin Mills over 5 years ago
Right. I think we're in agreement here. The ICC code could be causing a leak. It shouldn't of course, however for sure it could. I think auto pointers are a horrible idea. Never too sure who owns the pointer - grrrr.
Incidentally I think SVN is always defined. If the build doesn't know, it sets it to 0.
I really don't know anything about your plugin or gexiv2. It's a useful additional test. Let's do our work with the exiv2 sample apps because we both have them. I created Stonehenge2.webp with the cwebp:
681 rmills@rmillsmbp:~/temp/forBen $ curl --silent -O http://clanmills.com/Stonehenge.jpg 682 rmills@rmillsmbp:~/temp/forBen $ ls -alt Stonehenge.jpg -rw-r--r--+ 1 rmills staff 6757827 10 Aug 20:52 Stonehenge.jpg 683 rmills@rmillsmbp:~/temp/forBen $ cwebp -metadata all Stonehenge.jpg -o Stonehenge2.webp Saving file 'Stonehenge2.webp' File: Stonehenge.jpg Dimension: 6000 x 4000 Output: 1936490 bytes Y-U-V-All-PSNR 38.52 45.95 46.86 39.94 dB block count: intra4: 61816 intra16: 31934 (-> 34.06%) skipped block: 15202 (16.22%) bytes used: header: 370 (0.0%) mode-partition: 302819 (15.6%) Residuals bytes |segment 1|segment 2|segment 3|segment 4| total macroblocks: | 5%| 21%| 22%| 50%| 93750 quantizer: | 36 | 34 | 29 | 19 | filter level: | 11 | 8 | 6 | 8 | Metadata: * EXIF data: 15280 bytes * XMP data: 2579 bytes 684 rmills@rmillsmbp:~/temp/forBen $ exiv2 -pS Stonehenge2.webp STRUCTURE OF WEBP FILE: Stonehenge2.webp Chunk | Length | Offset | Payload RIFF | 1954376 | 0 | WEBP VP8X | 10 | 12 | ....o.... VP8 | 1936470 | 30 | .....*p...>.D.I..#$(.*p...gn..d. EXIF | 15280 | 1936508 | II*............................ XMP | 2579 | 1951796 | <?xpacket begin="..." id="W5M0Mp 685 rmills@rmillsmbp:~/temp/forBen $ exiv2 -pa Stonehenge2.webp Warning: Failed to decode Exif metadata. Exif.Image.DateTime Ascii 20 2015:07:16 20:25:28 Exif.Image.ImageDescription Ascii 13 Classic View Exif.Image.Rating Short 1 0 Iptc.Application2.RecordVersion Short 1 0 Iptc.Envelope.CharacterSet String 3 G Iptc.Application2.Caption String 12 Classic View Xmp.xmp.Rating XmpText 1 0 Xmp.xmp.ModifyDate XmpText 25 2015-07-16T20:25:28+01:00 Xmp.dc.description LangAlt 1 lang="x-default" Classic View 686 rmills@rmillsmbp:~/temp/forBen $
Updated by Ben Touchette over 5 years ago
I'm starting to distrust cwebp because it fails to create lossless images as well as the error above. I think i found a work around for fetching the height by getting it from the first available frame chunk (Vp8/Vp8l/Anmf).
Updated by Ben Touchette over 5 years ago
We can also create proper webp files using webpmux
eg.
cwebp Stonehenge.jpg -o Stonehenge.webp
webpmux -set exif Stonehenge.exv Stonehenge.webp -o Stonehenge-exif.webp
webpmux -set xmp Stonehenge.xmp Stonehenge-exif.webp -o Stonehenge-exif-xmp.webp
webpmux -set icc Stonehenge.icc Stonehenge-exif-xmp.webp -o Stonehenge-exif-xmp-icc.webp
Updated by Ben Touchette over 5 years ago
- File exiv2-height-160810.diff exiv2-height-160810.diff added
Patch to fix listed height issue on VP8L chunks (lossless single frame/image)
Updated by Robin Mills over 5 years ago
- File Stonehenge3.webp Stonehenge3.webp added
Ben
I've merged your patch and made some minor changes. Please sync and review. r4363
1 Stonehenge3.webp
I've tried your tricks above with cwebp and webpmux and created Stonehenge3.webp (attached). Your new code is reporting the metadata.
732 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa --grep model/i ~/Stonehenge3.webp Error: XMP Toolkit error 201: XML parsing failure Warning: Failed to decode XMP metadata. Exif.Image.Model Ascii 12 NIKON D5300 733 rmills@rmillsmbp:~/gnu/exiv2/trunk $A couple of grumpy warnings for you to investigate.
2 Test files
I found some here. https://github.com/drewnoakes/metadata-extractor-images/tree/master/webp
742 rmills@rmillsmbp:~/gnu/exiv2/trunk $ ls -alt ~/Downloads/*.webp -rw-r--r--@ 1 rmills staff 19944 11 Aug 09:46 /Users/rmills/Downloads/Nikon D1X.webp -rw-r--r--@ 1 rmills staff 273410 11 Aug 09:45 /Users/rmills/Downloads/webpvp8.webp -rw-r--r--@ 1 rmills staff 474772 11 Aug 09:43 /Users/rmills/Downloads/Nikon Coolpix P7000.webp 743 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 --verbose -pa --grep model/i ~/Downloads/*.webp File 1/3: /Users/rmills/Downloads/Nikon Coolpix P7000.webp Warning: Failed to decode Exif metadata. /Users/rmills/Downloads/Nikon Coolpix P7000.webp Exif.Image.Model Ascii 14 COOLPIX P7000 /Users/rmills/Downloads/Nikon Coolpix P7000.webp Xmp.tiff.Model XmpText 13 COOLPIX P7000 File 2/3: /Users/rmills/Downloads/Nikon D1X.webp Warning: Failed to decode Exif metadata. /Users/rmills/Downloads/Nikon D1X.webp: No Exif data found in the file /Users/rmills/Downloads/Nikon D1X.webp: No IPTC data found in the file /Users/rmills/Downloads/Nikon D1X.webp: No XMP data found in the file File 3/3: /Users/rmills/Downloads/webpvp8.webp /Users/rmills/Downloads/webpvp8.webp: No Exif data found in the file /Users/rmills/Downloads/webpvp8.webp: No IPTC data found in the file /Users/rmills/Downloads/webpvp8.webp: No XMP data found in the file 744 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 --verbose -pS ~/Downloads/*.webp File 1/3: /Users/rmills/Downloads/Nikon Coolpix P7000.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Nikon Coolpix P7000.webp Chunk | Length | Offset | Payload RIFF | 474764 | 0 | WEBP VP8X | 10 | 12 | ....?.... VP8 | 410596 | 30 | p.&..*@...>.H.K..).#r..@..gn..g. EXIF | 59965 | 410634 | II*........... ................ XMP | 4155 | 470608 | <?xpacket begin="..." id="W5M0Mp File 2/3: /Users/rmills/Downloads/Nikon D1X.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Nikon D1X.webp Chunk | Length | Offset | Payload RIFF | 19936 | 0 | WEBP VP8X | 10 | 12 | ....W.... VP8 | 16128 | 30 | .c...*X...>.D.J...$..+p...g:.7.. EXIF | 3770 | 16166 | II*........... ................ File 3/3: /Users/rmills/Downloads/webpvp8.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/webpvp8.webp Chunk | Length | Offset | Payload RIFF | 273402 | 0 | WEBP VP8 | 273390 | 12 | 0 ...*@...>U$.E#.!..Q.Hp..em3..q 745 rmills@rmillsmbp:~/gnu/exiv2/trunk $Our behaviour is a bit random.
3 webpmux is not good for creating test files
The problem is that webpmux doesn't know anything about metadata. It simply copies any data you give him into a chunk in the file.
4 What I'm going to do now
I'm going to focus on printStructure on the following three files.
749 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 --verbose -pR ~/Downloads/N*.webp ~/Stonehenge2.webp File 1/3: /Users/rmills/Downloads/Nikon Coolpix P7000.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Nikon Coolpix P7000.webp Chunk | Length | Offset | Payload RIFF | 474764 | 0 | WEBP VP8X | 10 | 12 | ....?.... VP8 | 410596 | 30 | p.&..*@...>.H.K..).#r..@..gn..g. EXIF | 59965 | 410634 | II*........... ................ STRUCTURE OF TIFF FILE (II): /Users/rmills/Downloads/Nikon Coolpix P7000.webp dirLength = 17751 END /Users/rmills/Downloads/Nikon Coolpix P7000.webp XMP | 4155 | 470608 | <?xpacket begin="..." id="W5M0Mp File 2/3: /Users/rmills/Downloads/Nikon D1X.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Nikon D1X.webp Chunk | Length | Offset | Payload RIFF | 19936 | 0 | WEBP VP8X | 10 | 12 | ....W.... VP8 | 16128 | 30 | .c...*X...>.D.J...$..+p...g:.7.. EXIF | 3770 | 16166 | II*........... ................ STRUCTURE OF TIFF FILE (II): /Users/rmills/Downloads/Nikon D1X.webp dirLength = 17751 END /Users/rmills/Downloads/Nikon D1X.webp File 3/3: /Users/rmills/Stonehenge2.webp STRUCTURE OF WEBP FILE: /Users/rmills/Stonehenge2.webp Chunk | Length | Offset | Payload RIFF | 1954376 | 0 | WEBP VP8X | 10 | 12 | ....o.... VP8 | 1936470 | 30 | .....*p...>.D.I..#$(.*p...gn..d. EXIF | 15280 | 1936508 | II*............................ STRUCTURE OF TIFF FILE (II): /Users/rmills/Stonehenge2.webp dirLength = 17751 END /Users/rmills/Stonehenge2.webp XMP | 2579 | 1951796 | <?xpacket begin="..." id="W5M0Mp 750 rmills@rmillsmbp:~/gnu/exiv2/trunk $
5 Null padding blocks with odd length
I believe that's in the spec. I can't remember where I read that, however I'm confident that I read that on the libwebp web site.
Updated by Ben Touchette over 5 years ago
1- The XMP chunk is filled with exif data. Probably why it fails like that :)
2- Nice, when i googled originally all i got back from the search results was total crap. Will fetch them and give them the once over and see whats there.
3- cwebp uses the same tricks to do it. But with webpmux you can create some specific files with any data.
4- Sounds better, i should have done that for the header part but i had a lot of other things on my mind. Going to sync up the code here with the repo after my first coffee.
5- Really i only found out after a lot of time reverse engineering a bunch of files with a hexeditor, it would have saved me considerable time if i had known before hand. Google really really sucks at documenting their APIs. sigh.
Updated by Ben Touchette over 5 years ago
Just discovered i can use svn to get a single subdirectory from any github project. In this case like so:
svn checkout https://github.com/drewnoakes/metadata-extractor-images/trunk/webp
Didn't know github actually support svn lol.
Updated by Ben Touchette over 5 years ago
Only these 3 files have metadata from that repo:
HTC Desire.webp (exif)
Nikon Coolpix P7000.webp (exif,xmp)
Nikon D1X.webp (exif)
Updated by Ben Touchette over 5 years ago
I think i figured out why the warning about decoding exif; which has to do with the test for the header. Not sure how i'll tackle it just now.
Updated by Robin Mills over 5 years ago
- % Done changed from 40 to 50
Ah. I stumbled onto two of the three files by pure luck. -pR / -pX are working and tested. -pC is in the code and waiting for a test file. r4366
If I spend time on Exiv2 tomorrow, I will get murdered. I'll check email/Redmine/Skype. I'll try not to get sucked into anything.
886 rmills@rmillsmbp:~/gnu/exiv2/trunk $ ls -alt ~/Downloads/N*.webp -rw-r--r--@ 1 rmills staff 19944 11 Aug 09:46 /Users/rmills/Downloads/Nikon D1X.webp -rw-r--r--@ 1 rmills staff 474772 11 Aug 09:43 /Users/rmills/Downloads/Nikon Coolpix P7000.webp 887 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pR ~/Downloads/N*.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Nikon Coolpix P7000.webp Chunk | Length | Offset | Payload RIFF | 474764 | 0 | WEBP VP8X | 10 | 12 | ....?.... VP8 | 410596 | 30 | p.&..*@...>.H.K..).#r..@..gn..g. EXIF | 59965 | 410634 | II*........... ................ STRUCTURE OF TIFF FILE (II): MemIo address | tag | type | count | offset | value 10 | 0x010e ImageDescription | ASCII | 32 | 146 | 22 | 0x010f Make | ASCII | 6 | 178 | NIKON 34 | 0x0110 Model | ASCII | 16 | 184 | COOLPIX P7000.. 46 | 0x0112 Orientation | SHORT | 1 | 6 | 6 58 | 0x011a XResolution | RATIONAL | 1 | 200 | 200/0 70 | 0x011b YResolution | RATIONAL | 1 | 208 | 208/0 82 | 0x0128 ResolutionUnit | SHORT | 1 | 2 | 2 94 | 0x0131 Software | ASCII | 32 | 216 | COOLPIX P7000V1.0 106 | 0x0132 DateTime | ASCII | 20 | 248 | 2010:10:21 11:22:48 118 | 0x0213 YCbCrPositioning | SHORT | 1 | 2 | 2 130 | 0x8769 ExifTag | LONG | 1 | 268 | 268 STRUCTURE OF TIFF FILE (II): MemIo address | tag | type | count | offset | value 270 | 0x829a ExposureTime | RATIONAL | 1 | 694 | 694/0 282 | 0x829d FNumber | RATIONAL | 1 | 702 | 702/0 294 | 0x8822 ExposureProgram | SHORT | 1 | 2 | 2 306 | 0x8827 ISOSpeedRatings | SHORT | 1 | 100 | 100 318 | 0x9000 ExifVersion | UNDEFINED | 4 | 808596016 | 0220 330 | 0x9003 DateTimeOriginal | ASCII | 20 | 710 | 2010:10:21 11:22:48 342 | 0x9004 DateTimeDigitized | ASCII | 20 | 730 | 2010:10:21 11:22:48 354 | 0x9101 ComponentsConfiguration | UNDEFINED | 4 | 197121 | ... 366 | 0x9102 CompressedBitsPerPixel | RATIONAL | 1 | 750 | 750/0 378 | 0x9204 ExposureBiasValue | SRATIONAL | 1 | 758 | 758/0 390 | 0x9205 MaxApertureValue | RATIONAL | 1 | 766 | 766/0 402 | 0x9207 MeteringMode | SHORT | 1 | 5 | 5 414 | 0x9208 LightSource | SHORT | 1 | 0 | 0 426 | 0x9209 Flash | SHORT | 1 | 16 | 16 438 | 0x920a FocalLength | RATIONAL | 1 | 774 | 774/0 450 | 0x927c MakerNote | UNDEFINED | 4721 | 1040 | Nikon.....II*.....+............ ... STRUCTURE OF TIFF FILE (II): MemIo address | tag | type | count | offset | value 10 | 0x0001 Version | UNDEFINED | 4 | 512 | ... 22 | 0x0002 ISOSpeed | SHORT | 2 | 0 | 0 0 34 | 0x0003 ColorMode | ASCII | 7 | 530 | COLOR 46 | 0x0004 Quality | ASCII | 8 | 538 | FINE 58 | 0x0005 WhiteBalance | ASCII | 14 | 546 | AUTO 70 | 0x0006 Sharpening | ASCII | 8 | 560 | NORMAL 82 | 0x0007 Focus | ASCII | 8 | 568 | AF-S 94 | 0x0008 FlashSetting | ASCII | 8 | 576 | 106 | 0x000a 0x000a | RATIONAL | 1 | 584 | 584/0 118 | 0x000b ProcessingSoftware | SSHORT | 2 | 0 | 0 0 130 | 0x000f ISOSelection | ASCII | 8 | 592 | AUTO 142 | 0x0010 DataDump | UNDEFINED | 3342 | 600 | ...............1.0.......#b6#... ... 154 | 0x0011 GPSImgDirection | LONG | 1 | 4632 | 4632 166 | 0x001a GPSDestDistance | ASCII | 40 | 3942 | . ... 178 | 0x0021 | UNDEFINED | 106 | 3982 | ............................... ... 190 | 0x0022 | SHORT | 1 | 0 | 0 202 | 0x0026 | SHORT | 18 | 4088 | 256 0 0 0 0 ... 214 | 0x0027 | SHORT | 7 | 4124 | 256 320 240 0 0 ... 226 | 0x0029 | SHORT | 2 | 0 | 0 0 238 | 0x002d | SHORT | 2 | 256 | 256 0 250 | 0x002e | SHORT | 1 | 0 | 0 262 | 0x002f | SHORT | 1 | 0 | 0 274 | 0x0030 | SHORT | 1 | 0 | 0 286 | 0x0080 ImageAdjustment | ASCII | 14 | 4138 | NORMAL 298 | 0x0081 | ASCII | 10 | 4152 | NORMAL . 310 | 0x0082 AuxiliaryLens | ASCII | 14 | 4162 | OFF 322 | 0x0085 FocusDistance | RATIONAL | 1 | 4176 | 4176/0 334 | 0x0086 DigitalZoom | RATIONAL | 1 | 4184 | 4184/0 346 | 0x0088 AFFocusPos | UNDEFINED | 4 | 768 | ... 358 | 0x008f | ASCII | 16 | 4192 | 370 | 0x0091 | UNDEFINED | 225 | 4208 | ...DTVR................g.d..... ... 382 | 0x0094 | SSHORT | 1 | 0 | 0 394 | 0x0095 | ASCII | 6 | 4434 | OFF 406 | 0x009b | BYTE | 2 | 0 | . 418 | 0x009c | ASCII | 20 | 4440 | 430 | 0x009d | SHORT | 1 | 0 | 0 442 | 0x009e | SHORT | 10 | 4460 | 0 0 0 0 0 ... 454 | 0x00a8 | UNDEFINED | 46 | 4480 | 0104.B......................... ... 466 | 0x00aa | ASCII | 16 | 4526 | NORMAL 478 | 0x00ac | ASCII | 14 | 4542 | VR-ON . 490 | 0x00b2 | ASCII | 10 | 4556 | NORMAL . 502 | 0x00b3 | ASCII | 8 | 4566 | 514 | 0x00bd | UNDEFINED | 58 | 4574 | 0100STANDARD .........STANDARD ... END MemIo 462 | 0x9286 UserComment | UNDEFINED | 126 | 782 | ........ ... 474 | 0xa000 FlashpixVersion | UNDEFINED | 4 | 808464688 | 0100 486 | 0xa001 ColorSpace | SHORT | 1 | 1 | 1 498 | 0xa002 PixelXDimension | SHORT | 1 | 3648 | 3648 510 | 0xa003 PixelYDimension | SHORT | 1 | 2736 | 2736 522 | 0xa005 InteroperabilityTag | LONG | 1 | 1010 | 1010 534 | 0xa300 FileSource | UNDEFINED | 1 | 3 | . 546 | 0xa301 SceneType | UNDEFINED | 1 | 1 | . 558 | 0xa401 CustomRendered | SHORT | 1 | 0 | 0 570 | 0xa402 ExposureMode | SHORT | 1 | 0 | 0 582 | 0xa403 WhiteBalance | SHORT | 1 | 0 | 0 594 | 0xa404 DigitalZoomRatio | RATIONAL | 1 | 908 | 908/0 606 | 0xa405 FocalLengthIn35mmFilm | SHORT | 1 | 28 | 28 618 | 0xa406 SceneCaptureType | SHORT | 1 | 0 | 0 630 | 0xa407 GainControl | SHORT | 1 | 0 | 0 642 | 0xa408 Contrast | SHORT | 1 | 0 | 0 654 | 0xa409 Saturation | SHORT | 1 | 0 | 0 666 | 0xa40a Sharpness | SHORT | 1 | 0 | 0 678 | 0xa40c SubjectDistanceRange | SHORT | 1 | 0 | 0 END MemIo 918 | 0x0103 Compression | SHORT | 1 | 6 | 6 930 | 0x011a XResolution | RATIONAL | 1 | 994 | 994/0 942 | 0x011b YResolution | RATIONAL | 1 | 1002 | 1002/0 954 | 0x0128 ResolutionUnit | SHORT | 1 | 2 | 2 966 | 0x0201 JPEGInterchangeFormat | LONG | 1 | 8692 | 8692 978 | 0x0202 JPEGInterchangeFormatLeng | LONG | 1 | 6044 | 6044 END MemIo XMP | 4155 | 470608 | <?xpacket begin="..." id="W5M0Mp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Nikon D1X.webp Chunk | Length | Offset | Payload RIFF | 19936 | 0 | WEBP VP8X | 10 | 12 | ....W.... VP8 | 16128 | 30 | .c...*X...>.D.J...$..+p...g:.7.. EXIF | 3770 | 16166 | II*........... ................ STRUCTURE OF TIFF FILE (II): MemIo address | tag | type | count | offset | value 10 | 0x010e ImageDescription | ASCII | 32 | 170 | 22 | 0x010f Make | ASCII | 18 | 202 | NIKON CORPORATION 34 | 0x0110 Model | ASCII | 10 | 220 | NIKON D1X 46 | 0x011a XResolution | RATIONAL | 1 | 230 | 230/0 58 | 0x011b YResolution | RATIONAL | 1 | 238 | 238/0 70 | 0x0128 ResolutionUnit | SHORT | 1 | 2 | 2 82 | 0x0131 Software | ASCII | 10 | 246 | Ver.5.01 94 | 0x0132 DateTime | ASCII | 20 | 256 | 2003:08:06 18:04:34 106 | 0x0213 YCbCrPositioning | SHORT | 1 | 2 | 2 118 | 0x0214 ReferenceBlackWhite | RATIONAL | 6 | 276 | 0/0 1/0 255/0 1/0 128/0 ... 130 | 0x8298 Copyright | ASCII | 33 | 324 | Copyright, ... 142 | 0x8769 ExifTag | LONG | 1 | 358 | 358 STRUCTURE OF TIFF FILE (II): MemIo address | tag | type | count | offset | value 360 | 0x829a ExposureTime | RATIONAL | 1 | 872 | 872/0 372 | 0x829d FNumber | RATIONAL | 1 | 880 | 880/0 384 | 0x8822 ExposureProgram | SHORT | 1 | 2 | 2 396 | 0x9000 ExifVersion | UNDEFINED | 4 | 808596016 | 0220 408 | 0x9003 DateTimeOriginal | ASCII | 20 | 808 | 2003:08:06 18:04:34 420 | 0x9004 DateTimeDigitized | ASCII | 20 | 828 | 2003:08:06 18:04:34 432 | 0x9101 ComponentsConfiguration | UNDEFINED | 4 | 197121 | ... 444 | 0x9204 ExposureBiasValue | SRATIONAL | 1 | 848 | 848/0 456 | 0x9205 MaxApertureValue | RATIONAL | 1 | 856 | 856/0 468 | 0x9207 MeteringMode | SHORT | 1 | 5 | 5 480 | 0x9208 LightSource | SHORT | 1 | 4 | 4 492 | 0x9209 Flash | SHORT | 1 | 7 | 7 504 | 0x920a FocalLength | RATIONAL | 1 | 864 | 864/0 516 | 0x927c MakerNote | UNDEFINED | 2787 | 888 | Nikon.....II*...............0200 ... STRUCTURE OF TIFF FILE (II): MemIo address | tag | type | count | offset | value 10 | 0x0001 Version | UNDEFINED | 4 | 808464944 | 0200 22 | 0x0002 ISOSpeed | SHORT | 2 | 13107200 | 0 200 34 | 0x0003 ColorMode | ASCII | 6 | 386 | COLOR 46 | 0x0004 Quality | ASCII | 8 | 392 | RAW 58 | 0x0005 WhiteBalance | ASCII | 13 | 400 | FLASH 70 | 0x0006 Sharpening | ASCII | 7 | 414 | NONE 82 | 0x0007 Focus | ASCII | 7 | 422 | AF-S 94 | 0x0008 FlashSetting | ASCII | 13 | 430 | NORMAL 106 | 0x0009 GPSStatus | ASCII | 9 | 444 | NEW_TTL 118 | 0x000b ProcessingSoftware | SSHORT | 1 | 0 | 0 130 | 0x000c GPSSpeedRef | RATIONAL | 4 | 454 | 15673/3582 57600/1525 26513/17 16960/15 142 | 0x000d GPSSpeed | UNDEFINED | 4 | 196864 | ... 154 | 0x000e GPSTrackRef | UNDEFINED | 4 | 786688 | ... 166 | 0x0011 GPSImgDirection | LONG | 1 | 2781 | 2781 178 | 0x0081 | ASCII | 7 | 486 | NORMAL 190 | 0x0083 | BYTE | 1 | 2 | . 202 | 0x0084 | RATIONAL | 4 | 494 | 17/0 1/0 35/0 1/0 214 | 0x0087 | BYTE | 1 | 3 | . 226 | 0x0088 AFFocusPos | UNDEFINED | 4 | 65537 | ... 238 | 0x0089 | BYTE | 1 | 1 | . 250 | 0x008b | UNDEFINED | 4 | 786760 | H.. 262 | 0x008c | UNDEFINED | 2112 | 526 | ............................... ... 274 | 0x008d | ASCII | 9 | 2638 | MODE2 286 | 0x008e | SRATIONAL | 4 | 2648 | 27885/840 57600/1525 34979/65455 57600/1525 298 | 0x0090 | ASCII | 12 | 2680 | SPEEDLIGHT 310 | 0x0091 | UNDEFINED | 53 | 2692 | 0100....%6... .....F...........4 ... 322 | 0x0092 | SSHORT | 1 | 3 | 3 334 | 0x0098 | UNDEFINED | 20 | 2745 | 0100w\cH+D$$h.$.5.Q 346 | 0x0099 | SHORT | 2 | 43386846 | 2014 662 358 | 0x009a | RATIONAL | 2 | 2765 | 59/0 10/0 370 | 0x0e10 | LONG | 1 | 2875 | 2875 END MemIo 528 | 0x9286 UserComment | UNDEFINED | 48 | 3676 | ............................... ... 540 | 0x9290 SubSecTime | ASCII | 3 | 12598 | 61 552 | 0x9291 SubSecTimeOriginal | ASCII | 3 | 12598 | 61 564 | 0x9292 SubSecTimeDigitized | ASCII | 3 | 12598 | 61 576 | 0xa000 FlashpixVersion | UNDEFINED | 4 | 808464688 | 0100 588 | 0xa001 ColorSpace | SHORT | 1 | 1 | 1 600 | 0xa002 PixelXDimension | LONG | 1 | 600 | 600 612 | 0xa003 PixelYDimension | LONG | 1 | 391 | 391 624 | 0xa217 SensingMethod | SHORT | 1 | 2 | 2 636 | 0xa300 FileSource | UNDEFINED | 1 | 3 | . 648 | 0xa301 SceneType | UNDEFINED | 1 | 1 | . 660 | 0xa302 CFAPattern | UNDEFINED | 8 | 3724 | ....... 672 | 0xa401 CustomRendered | SHORT | 1 | 0 | 0 684 | 0xa402 ExposureMode | SHORT | 1 | 0 | 0 696 | 0xa403 WhiteBalance | SHORT | 1 | 1 | 1 708 | 0xa404 DigitalZoomRatio | RATIONAL | 1 | 3732 | 3732/0 720 | 0xa405 FocalLengthIn35mmFilm | SHORT | 1 | 25 | 25 732 | 0xa406 SceneCaptureType | SHORT | 1 | 0 | 0 744 | 0xa407 GainControl | SHORT | 1 | 0 | 0 756 | 0xa408 Contrast | SHORT | 1 | 0 | 0 768 | 0xa409 Saturation | SHORT | 1 | 0 | 0 780 | 0xa40a Sharpness | SHORT | 1 | 1 | 1 792 | 0xa40c SubjectDistanceRange | SHORT | 1 | 0 | 0 END MemIo 154 | 0x8825 GPSTag | LONG | 1 | 3740 | 3740 END MemIo 888 rmills@rmillsmbp:~/gnu/exiv2/trunk $
Updated by Ben Touchette over 5 years ago
Gotcha and nice :)
Good luck on that other matter. I'll be updating the decodeChunk function shortly i think i have all the kinks out now. Doing one last pass before committing the code in.
Updated by Robin Mills over 5 years ago
Never worry about making mistakes and checking in code that's not right. The whole point of our test harness is to detect that. Of course, we haven't got any tests for your stuff yet, however nobody knows it's there anyway. And we're on the trunk which is a development branch. I try to keep it stable so that it will always build and pass our test suite.
I've drawn a blank on #1203. I'll deal with it in v0.27 next year. I closed #1202 today and printStructure() is working. A good day.
Updated by Ben Touchette over 5 years ago
Just read the for #1203, yep sounds a like a good one. As for the commits i'm not too worried but i prefer to do the work to fix stuff (if possible) before shipping it :)
I cleaned up a bunch of stuff while i was in there fix it. Oh and created a hexdump function that probably should go elsewhere in the but we can discuss that in chat. Prints two 8 byte wide hex sections and the ascii dump to the right of that. I needed that instead of opening the hex editor every two seconds to get the proper info from some of the chunks. Anyways it'll be in this commit.
Updated by Robin Mills over 5 years ago
This looks fine, Ben. A couple of suggestions:
1 Move the hexDump code to the Internal namespace
We'll find a good use for this elsewhere in the code. Much as I have done for Internal::binaryToString(). I suggest a signature such as:
std::string Internal::binaryToHex(const byte*,size_t);
2 I'm surprised that you can parse Stonehenge3.webp
Is this what you refer to as a "long" header? I have my doubts that Stonehenge3.webp is a valid file. The EXIF data in a JPEG is stored in an APP1 segment and it's a 100% valid TIFF file. I think the WebP EXIF Chunk is a valid TIFF file (although it has no image). You can extract the TIFF with the utility dd (dastardly dangerous). dd is one of the most feared tools in Linux.
900 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS ~/Stonehenge2.webp STRUCTURE OF WEBP FILE: /Users/rmills/Stonehenge2.webp Chunk | Length | Offset | Payload RIFF | 1954376 | 0 | WEBP VP8X | 10 | 12 | ....o.... VP8 | 1936470 | 30 | .....*p...>.D.I..#$(.*p...gn..d. EXIF | 15280 | 1936508 | II*............................ XMP | 2579 | 1951796 | <?xpacket begin="..." id="W5M0Mp 901 rmills@rmillsmbp:~/gnu/exiv2/trunk $ dd bs=1 skip=$((1936508+8)) count=15280 if=~/Stonehenge2.webp of=Stonehenge2.tiff 15280+0 records in 15280+0 records out 15280 bytes transferred in 0.055026 secs (277688 bytes/sec) 902 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pR Stonehenge2.tiff STRUCTURE OF TIFF FILE (II): Stonehenge2.tiff address | tag | type | count | offset | value 10 | 0x010f Make | ASCII | 18 | 146 | NIKON CORPORATION 22 | 0x0110 Model | ASCII | 12 | 164 | NIKON D5300 ... 4410 | 0x0213 YCbCrPositioning | SHORT | 1 | 1 | 1 END Stonehenge2.tiff 903 rmills@rmillsmbp:~/gnu/exiv2/trunk $The following command fails miserably:
904 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pR ~/Stonehenge3.webp STRUCTURE OF WEBP FILE: /Users/rmills/Stonehenge3.webp Chunk | Length | Offset | Payload RIFF | 1972496 | 0 | WEBP VP8X | 10 | 12 | ....o.... VP8 | 1936470 | 30 | .....*p...>.D.I..#$(.*p...gn..d. EXIF | 17989 | 1936508 | ..Exiv2..;.Exif..II*............ STRUCTURE OF TIFF FILE (??): MemIo END MemIo XMP | 17989 | 1954506 | ..Exiv2..;.Exif..II*............ 905 rmills@rmillsmbp:~/gnu/exiv2/trunkI could fix that of course. However I don't think I should fix it. Stonehenge3.webp has an embedded .exv file in the EXIF Chunk. I don't think that's valid use of the EXIF Chunk in WebP.
905 rmills@rmillsmbp:~/gnu/exiv2/trunk $ dd bs=1 skip=$((1936508+8)) count=17989 if=~/Stonehenge3.webp of=Stonehenge3.exv 17989+0 records in 17989+0 records out 17989 bytes transferred in 0.053564 secs (335841 bytes/sec) 906 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pR Stonehenge3.exv STRUCTURE OF JPEG FILE: Stonehenge3.exv address | marker | length | data 2 | 0x01 | 0 9 | 0xe1 APP1 | 15296 | Exif..II*...................... STRUCTURE OF TIFF FILE (II): MemIo address | tag | type | count | offset | value 10 | 0x010f Make | ASCII | 18 | 146 | NIKON CORPORATION 22 | 0x0110 Model | ASCII | 12 | 164 | NIKON D5300 ... 4394 | 0x0201 JPEGInterchangeFormat | LONG | 1 | 4450 | 4450 4406 | 0x0202 JPEGInterchangeFormatLeng | LONG | 1 | 10837 | 10837 4418 | 0x0213 YCbCrPositioning | SHORT | 1 | 1 | 1 END MemIo 15307 | 0xe1 APP1 | 2610 | http://ns.adobe.com/xap/1.0/.<?x 17919 | 0xed APP13 | 68 | Photoshop 3.0.8BIM.......'..... Record | DataSet | Name | Length | Data 1 | 0 | ModelVersion | 2 | .. 1 | 90 | CharacterSet | 3 | .%G 2 | 0 | RecordVersion | 2 | .. 2 | 120 | Caption | 12 | Classic View 17989 | 0xd9 EOI 907 rmills@rmillsmbp:~/gnu/exiv2/trunk $3 File Type Recognition
Exiv2 doesn't use the file extension to recognise a file type. It 'sniffs' inside the file. The ImageFactory has an array of filesTypes. He opens the file an calls the registered sniffer. For WebP it's isWebPType. If you say "I'm good with this", you handle it. The file extension is irrelevant.
907 rmills@rmillsmbp:~/gnu/exiv2/trunk $ grep isWebPType src/*.cpp src/image.cpp: { ImageType::webp, newWebPInstance, isWebPType, amReadWrite, amNone, amReadWrite, amNone }, src/webpimage.cpp: if (!isWebPType(*io_, true)) { src/webpimage.cpp: if (!isWebPType(*io_, true)) { src/webpimage.cpp: bool isWebPType(BasicIo& iIo, bool /*advance*/) 908 rmills@rmillsmbp:~/gnu/exiv2/trunk $4 I'm going to do my chores today
I've got a load of stuff to get done around the house and garden. Alison is painting. If I spend all day on the computer, she might throw it out the window. I'll be checking email and Redmine - so please ask if I can help you in some way.
I've spent about half my working life as the project leader and folks often say something like "I don't want to disturb you. You might get mad if I waste your time!". And my response is usually. "You are never disturbing me. I will get mad if I discover that your wheels are spinning and you didn't ask for help. If your wheels are spinning for more than 2 hours, I want to know about it!"
Updated by Ben Touchette over 5 years ago
1- indeed i'll see what i can come up with.
2- hahaha, i use dd a lot, though for some tasks i now use pv when copying data as it gives me a progress bar. As for decoding i made it so it was more resilient with various forms of data i tried since the encoder (webpmux) doesn't care whats going in there i try to validate and preformat the data before feeding it to the exif decoder. That way it has a better chance of pulling out something useful. I could just let it fail though :)
3- Not sure what you mean here, i already have isWebPType sniffing the header to make sure its valid.
4- Sounds like a good idea :) and yeah that was a tough one early in my career, sadly some places and managers don't see it that way, you should already be an all knowing robot :/ Reminds me of this video i saw onj youtube last year, reminded me of quite a few meetings i was in on ... https://www.youtube.com/watch?v=BKorP55Aqvg
Updated by Robin Mills over 5 years ago
Your WebP file sniffer is working perfectly. I was explaining how it worked and if you already knew that, well that's fine.
The webpmux bastardo. I'll have to look at him. If he's generating horrible files, we should handle them. However if the file is bad, we should reject it. Exiv2 philosophy: we report/modify/write perfect files. We don't forgive deviation from the standard - that's a very slippery slope.
The video is a hoot. I have a nightmare a couple of times every week about being back at work and being asked to do impossible things (like drawing red lines with blue ink). No matter how I try to negotiate, I cannot get the stakeholders to change the request.
The other nightmare is that when the project is done, the boss redefines the finish line with something new. For example, when I finish the new kitchen, the boss says "well, that's kind of OK, however you know that the bathroom is more important. Why has the bathroom not been done and you know it must be finished for guests arriving on Sunday! You don't know about the guests because you were so busy on the stupid kitchen that you didn't attend the meeting where we discussed the guests. Your communication skills need a lot of work."
I never miss work. I've got Exiv2 to keep my software skills alive. I love my wife and we're very happy to be working together to make our house perfect. Life is good - never been happier.
Updated by Ben Touchette over 5 years ago
Ah ok i did know, figured it out after looking it the other file types when i started. Was momentarily confused, i blame it on not ingesting the "Coffee" before answering :)
Wepbmux just stuffs whatever it gets told to put in there i don't think they do any validation from the quick test i've done (actually just look at what it let through for the XMP chunk in Stonehenge3.webp for another example). When i encode the exif not sure i've got it just right, though it works now, it feels off.
I used to have similar dreams after my last official job. When saw that video the first i both laughed and cringed having been in similar situations.
Yeah its good to have projects, keeps the mind active and skills sharp. I'm glad for you and that's a good thing you get along with the 'boss' so well :)
Updated by Ben Touchette over 5 years ago
Just enabled the iptc handling and fixed the warnings in stringToHex.
Updated by Ben Touchette over 5 years ago
Wondering if i should enable copying the exif data to xmp?
Updated by Robin Mills over 5 years ago
I think you should ask Alan about this exif/xmp stuff. If we do that for jpg and tiff, we should do it for webp.
There are a couple of features of Exiv2 that I wish had never been added to the code base: Lens Recognition and conversion functions. The reason I dislike these features is that the metadata is "cooked up" by the code and not in the file itself. Both are troublesome. Lens Recognition is probably 50% of the issues reported in Redmine. Because of my background in compilers/interpreters, I never try to guess what the code being compiled might mean. If it's a syntax violation, I signal an error and leave the human to deal with it.
Updated by Ben Touchette over 5 years ago
I'll double check whats done there and ask if its not clear.
Is there somewhere, or resource i could read up on lens recognition (aside from the code lol)? I know that past versions of Photoshop also had issues with one of my lenses when doing lens correction in Camera Raw. I'd have to try it with a recent version of it.
I just dislike messy warnings of any kind, i find it irritating, not that i always correct them but yeah :)
Updated by Robin Mills over 5 years ago
There's not much to the Lens Recognition. It's easy to understand. The manufacturer is meant to set a unique LensId in the metadata
938 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa --grep LensID/i ~/Stonehenge.jpg Exif.NikonLd3.LensIDNumber Byte 1 Sigma 18-250mm F3.5-6.3 DC OS Macro HSM 939 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pv --grep LensID/i ~/Stonehenge.jpg 0x000c NikonLd3 LensIDNumber Byte 1 146 940 rmills@rmillsmbp:~/gnu/exiv2/trunk $There are two horrible consequences:
1) Every manufacturer uses a different way to store this. For Nikon, it's Exif.NikonLd3.LensIDNumber, Sony it's Exif.Sony1.LensID
2) Often, the same ID is used for several different lenses, so we need to inspect the other metadata to guess what lens is being used.
945 rmills@rmillsmbp:~/gnu/exiv2/trunk $ hex 146 146 = 0x92 946 rmills@rmillsmbp:~/gnu/exiv2/trunk $ grep 92 src/nikonmn.cpp TagInfo(0x0092, "HueAdjustment", N_("Hue Adjustment"), N_("Hue adjustment"), nikon3Id, makerTags, signedShort, -1, printValue), {0x8C,0x40,0x2D,0x53,0x2C,0x3C,0x8E,0x06,0x01,0x00,0x00, "Nikon", "JAA792DA", "AF-S DX Zoom-Nikkor 18-55mm f/3.5-5.6G ED"}, {0x90,0x3B,0x53,0x80,0x30,0x3C,0x92,0x0E,0x01,0x00,0x00, "Nikon", "JAA798DA", "AF-S DX VR Zoom-Nikkor 55-200mm f/4-5.6G IF-ED"}, {0x92,0x48,0x24,0x37,0x24,0x24,0x94,0x06,0x01,0x00,0x00, "Nikon", "JAA801DA", "AF-S Zoom-Nikkor 14-24mm f/2.8G ED"}, {0x92,0x39,0x2D,0x88,0x2C,0x40,0x4B,0x0E,0x01,0x00,0x00, "Sigma", "", "18-250mm F3.5-6.3 DC OS Macro HSM"}, 947 rmills@rmillsmbp:~/gnu/exiv2/trunk $The maintenance of non-unique lensIDs is very troublesome. Anybody using Exiv2 who goes and buys a nice new lens is very likely to discover that we don't know about it. They raise an issue on Redmine.
For v0.26, we have a great new feature. The user has a "config" file ~/.exiv2 which can be updated on his machine. http://dev.exiv2.org/issues/1034
I don't think we use those copy/convertor.cpp functions for other image types, so lets not use them in webp.
952 rmills@rmillsmbp:~/gnu/exiv2/trunk $ grep copyIptcToXmp src/*.cpp src/convert.cpp: void copyIptcToXmp(const IptcData& iptcData, XmpData& xmpData, const char *iptcCharset) src/webpimage.cpp: copyIptcToXmp(iptcData_, xmpData_); src/xmpsidecar.cpp: copyIptcToXmp(iptcData_, xmpData_); 953 rmills@rmillsmbp:~/gnu/exiv2/trunk $ grep copyExifToXmp src/*.cpp src/convert.cpp: void copyExifToXmp(const ExifData& exifData, XmpData& xmpData) src/xmpsidecar.cpp: copyExifToXmp(exifData_, xmpData_); 954 rmills@rmillsmbp:~/gnu/exiv2/trunk $Write, I think we're almost ready to start on the test code. I've got some stuff to do in the garden today. I can talk later on Skype (after 17:00/12:00) and explain it to you.
Updated by Ben Touchette over 5 years ago
Indeed, guess you'd have to inspect focal lengths as well to guesstimate it.
Sounds good, hopefully no interruptions like yesterday on my end :)
Updated by Ben Touchette over 5 years ago
- File Stonehenge-with-icc.webp Stonehenge-with-icc.webp added
Updated by Gilles Caulier over 5 years ago
Next week i will start to implement the digiKam WebP image loader based on libwebp. I will able to test your Exiv2 WebP handler in real...
Updated by Ben Touchette over 5 years ago
@Gilles: Sweet. So far it works with the implementation of the GIMP plugin for WebP.
Updated by Ben Touchette over 5 years ago
@Gilles: I forgot to add that the code is in file-webp under plugins in the gimp git repo.
Updated by Robin Mills over 5 years ago
- Assignee changed from Robin Mills to Ben Touchette
- % Done changed from 50 to 60
- Estimated time changed from 15.00 h to 50.00 h
Ben
I'm going to assign this issue to you because you're doing to hard work to make this happen. When the release notes for v0.26 are prepared you will rightfully be given the credit for bringing this feature to life. You've done a great job. Thanks very much. I'll continue to support you with this. I'll probably add the erase code on Monday to implement -dtgt where txt = { a , e , i , x, c, C }.
Updated by Ben Touchette over 5 years ago
@Robin: :) sounds fine, I'm glad i could help, and thanks for saying. I already tested the i,e, and x cases for deleting (-d option) that was my last patch before i spent most of the day trying to debrick the router. Fun times cough with serial interfaces/cables, tftp, and router innards lol. I believe it is very much muerte though :/
Updated by Thomas Beutlich over 5 years ago
This is just a reminder to not forget updating exiv2.1 on the new WEBP file format. Thanks!
Updated by Gilles Caulier over 5 years ago
digiKam 5.2.0 now support WebP image format in read write .
Tested with current implementation from Exiv2. Sound working fine for the moment.
https://www.flickr.com/photos/digikam/29032567885/
Updated by Ben Touchette over 5 years ago
@Gilles Nice. I'm trying to catch up on everything right now.
Updated by Ben Touchette over 5 years ago
lol second try.
@Thomas: Not sure what you mean there?
Updated by Thomas Beutlich over 5 years ago
I noticed that source:trunk/src/exiv2.1 contains the list of supported file formats which serves as the Wiki base. This file needs to be updated to consider new WEBP support.
Updated by Ben Touchette over 5 years ago
Started the process of updating the wiki to add webp information.
Updated by Robin Mills over 5 years ago
I don't believe the EXIF chunk in test/data/exiv2-bug1199.webp is valid. It think it should be a TIFF. I've based this believe on the EXIF in this file:
1199 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS ~/Downloads/Nikon\ D1X.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Nikon D1X.webp Chunk | Length | Offset | Payload RIFF | 19936 | 0 | WEBP VP8X | 10 | 12 | ....W.... VP8 | 16128 | 30 | .c...*X...>.D.J...$..+p...g:.7.. EXIF | 3770 | 16166 | II*........... ................ 1200 rmills@rmillsmbp:~/gnu/exiv2/trunk $The data in the current file is:
1156 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187536 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12050 | 172614 | ../.Exif..II*.................. XMP | 2864 | 184672 | <?xpacket begin="..." id="W5M0MpI believe the EXIF chunk has 10 bytes which should not be there.
I've extracted the metadata, shortened the EXIF chunk and reassembled the file as follows:
$ cp test/data/exiv2-bug1199.webp 1199.webp $ webpmux -get icc 1199.webp -o 1199.icc $ webpmux -get exif 1199.webp -o 1199.exif $ webpmux -get xmp 1199.webp -o 1199.xmp $ dd bs=1 count=$((12050-10)) skip=10 if=1199.exif > 1199.tiff $ webpmux -set exif 1199.tiff 1199.webp -o 1199-1.webp $ webpmux -set icc 1199.icc 1199-1.webp -o 1199-2.webp $ webpmux -set xmp 1199.xmp 1199-2.webp -o 1199-3.webp $ cp 1199-3.webp test/data/exiv2-bug1199.webpThe modified file test/data/exiv2-bug1199.webp is now 10 bytes shorter. I've updated the test suite appropriately. I've updated the Wiki/WEBP page http://dev.exiv2.org/projects/exiv2/wiki/The_Metadata_in_WEBP_files
1200 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS test/data/exiv2-bug1199.webp STRUCTURE OF WEBP FILE: test/data/exiv2-bug1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp 1201 rmills@rmillsmbp:~/gnu/exiv2/trunk $
Updated by Ben Touchette over 5 years ago
You may be right i just ran cwebp with all metadata conversion on Stonehenge.jpg and it has the same format, so i guess i'll adjust accordingly.
Updated by Ben Touchette over 5 years ago
Changed it to remove the header padding. Should produce correct exif data now. Attaching new test image.
Updated by Robin Mills over 5 years ago
This looks better. Comparing three files:
606 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS ~/Downloads/Stonehenge-with-icc.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Stonehenge-with-icc.webp Chunk | Length | Offset | Payload RIFF | 187536 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12050 | 172614 | ../.Exif..II*.................. XMP | 2864 | 184672 | <?xpacket begin="..." id="W5M0Mp 607 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS ~/Downloads/Stonehenge-with-icc-exif-xmp.webp STRUCTURE OF WEBP FILE: /Users/rmills/Downloads/Stonehenge-with-icc-exif-xmp.webp Chunk | Length | Offset | Payload RIFF | 180990 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 165706 | 598 | .....*.. .>1..B.! ...yX@..gn.E.. EXIF | 12068 | 166312 | II*........................... . XMP | 2602 | 178388 | <?xpacket begin="..." id="W5M0Mp 608 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS test/data/exiv2-bug1199.webp STRUCTURE OF WEBP FILE: test/data/exiv2-bug1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp 609 rmills@rmillsmbp:~/gnu/exiv2/trunk $The new file looks correctly formatted with the TIFF in the EXIF chunk. Oddly, the XMP and EXIF chunks are different. Dumping and comparing the metadata reveals:
609 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa ~/Downloads/Stonehenge-with-icc-exif-xmp.webp | sort > new.txt 610 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa test/data/exiv2-bug1199.webp | sort > test.txt 611 rmills@rmillsmbp:~/gnu/exiv2/trunk $ diff test.txt new.txt 13c13 < Exif.Image.DateTime Ascii 20 2016:08:13 10:54:16 --- > Exif.Image.DateTime Ascii 20 2016:08:18 06:50:07 168c168 < Exif.Thumbnail.JPEGInterchangeFormatLength Long 1 6861 --- > Exif.Thumbnail.JPEGInterchangeFormatLength Long 1 6889 171,172d170 < Iptc.Application2.Byline String 11 Robin Mills < Iptc.Application2.BylineTitle String 10 Stonehenge 174d171 < Iptc.Application2.ObjectName String 10 Stonehenge 612 rmills@rmillsmbp:~/gnu/exiv2/trunk $I think your new file is fine. I will not change test/data/exiv2-bug1199.webp at this time as I believe it is already working perfectly. Your new file contains slightly different metadata, however there's nothing substantially different.
I know that exiv2 -dC test/data/exiv2-bug1199.webp isn't working. I will fix that today.
Updated by Ben Touchette over 5 years ago
The difference is probably due to my removing the iptc conversion code to/from xmp. After a fitful night, decided to remove it, apps should handle that themselves and seeing as Google didn't included iptc support in the spec at this time i guess you were right
Updated by Ben Touchette over 5 years ago
Getting (error 32) "Setting IPTC metadata in WebP images is not supported" when doing exiv2 -i X test.webp (with file test.xml in the same directory) to insert an xml file.
Updated by Robin Mills over 5 years ago
- % Done changed from 60 to 70
r4409. I've fixed the code for exiv2 -dC image-path in WebPImage::writeMetadata(). Please review.
There a bug in -de (delete Exif). It appears to be deleting EXIF and ICCP.
The following is fine (delete ICC Profile):
726 rmills@rmillsmbp:~/gnu/exiv2/trunk $ cp test/data/exiv2-bug1199.webp 1199.webp 727 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp 728 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 --verbose -dC 1199.webp File 1/1: 1199.webp Erasing ICC Profile data from the file 729 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 186958 | 0 | WEBP VP8X | 10 | 12 | ......... VP8 | 172008 | 30 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172046 | II*........................... . XMP | 2864 | 184094 | <?xpacket begin="..." id="W5M0Mp 730 rmills@rmillsmbp:~/gnu/exiv2/trunk $The following is fine (delete XMP)
730 rmills@rmillsmbp:~/gnu/exiv2/trunk $ cp test/data/exiv2-bug1199.webp 1199.webp 731 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp 732 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 --verbose -dx 1199.webp File 1/1: 1199.webp Erasing XMP data from the file 733 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 184094 | 0 | WEBP VP8X | 10 | 12 | (........ VP8 | 172008 | 30 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172046 | II*........................... . 734 rmills@rmillsmbp:~/gnu/exiv2/trunk $Deleting EXIF also deletes ICC:
747 rmills@rmillsmbp:~/gnu/exiv2/trunk $ cp test/data/exiv2-bug1199.webp 1199.webp 748 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp 749 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 --verbose -de 1199.webp File 1/1: 1199.webp Erasing Exif data from the file 750 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 174918 | 0 | WEBP VP8X | 10 | 12 | $........ VP8 | 172008 | 30 | .G...*.. .>1..B.!..o.. ......].. XMP | 2864 | 172046 | <?xpacket begin="..." id="W5M0Mp 751 rmills@rmillsmbp:~/gnu/exiv2/trunk $Can you investigate this, please. If you don't fix it, I'll deal with it on Friday.
I will add tests to the test suite on Friday for -dC -dx -de
Updated by Ben Touchette over 5 years ago
Committed the updates and added inserting icc profiles as well to the commit.
Updated by Robin Mills over 5 years ago
Ben
Right. I've submitted some fixes. r4416
Testing always seems like a good idea to me. Here's the status:
1 Refactored bugfixes-test.sh into webp-test.sh
You can run "our tests" with $ make webp-test
and you don't to wait for the suite to grind through all the bugs
2 Issues with actions.cpp/actions.hpp
I've made some changes to this. There are two options for inserting XMP -ix and -iX:
a) $ exiv2 -iX foo.xxx
- this reads foo.xmp (which must be XML) and copies it into the foo.xxx
b) $ exiv2 -ix foo.xxx
(not working correctly, discussion below)
- this reads foo.exv and updates the XMP in foo.xxx with the XMP from foo.exv
- you can create foo.exv with $ exiv2 --force --verbose -ea foo.xxx
c) $ cat foo.xmp | exiv2 -ix- foo.xxx
(probably not working at all)
- the reads something from stdin and puts it somewhere
- we should get this to "sniff" the data in stdin
- this is off topic. Please don't spend time on this. I'll fix this soon.
3 Changes to webpimage.cpp
Have a look. Nothing major. Ask if you want me to explain any of the changes.
4 Moving forward
I'm a bit lost with this. If you'd like to investigate with the debugger. Or I'll fix it tomorrow.
1069 rmills@rmillsmbp:~/gnu/exiv2/trunk $ cp test/data/exiv2-bug1199.webp 1199.webp ; cp test/data/Reagan.tiff . ; exiv2 -ea --force Reagan.tiff ; mv Reagan.exv 1199.exf ; exiv2 -pS 1199.webp ; exiv2 -ix 1199.webp ; exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187241 | 0 | WEBP P8X | 738197504 | 13 | .........ICCP0......0ADBE....mnt <--- YIKES 1070 rmills@rmillsmbp:~/gnu/exiv2/trunk $
Updated by Ben Touchette over 5 years ago
1 - sweet, still need to figure that damned library issue i mentioned.
2ab - so many combinations i haven't explored yet :)
2c - does exiv2 validate xml?
4- I had that happening yesterday after your changes, the recast when writing data seemed to have been failing and basically wasn't writing anything so the offsets were wrong due to the chunk not having any data that it said it had. Probably something bad along those lines here, will give a once over.
Updated by Robin Mills over 5 years ago
The exiv2 utility is a feature rich thing. At the moment, I don't validate XMP, and perhaps I never will. If you want to put any load of binary code into the XMP storage location, I don't see any reason to prevent you!
There's definitely something not nice happening in WebPImage::writeMetadata(). We'll find it. We'll fix it. No question about it.
I'm rather happy to be working on this. It's brought me back to exploring this ix -iX -iX ix calculus. For sure I want to get all that magic functioning correctly for v0.26 and properly tested in the test suite.
I'm not surprised by the time that this is taking. I wanted to defer this to v0.27 and focus on getting v0.26 out the door. We'll get this fixed AND we'll get v0.26 finished. However, I know that very soon I will get an email from somebody saying "why isn't v0.26 finished yet?". I might not get paid, however there are many folks using Exiv2. One of them will be urgently waiting for one of the new features in v0.26.
Updated by Ben Touchette over 5 years ago
Indeed an intricate weave of functionality so of course there's lots of plumbing to work on lol.
I hear ya on the time-frame. People get funny about that even when it's freely given. At the time when i first submitted the patch i didn't realize that or the level of intricacy hidden behind exiv2, i figured at the time it was just going to be another quick hi/bye thanks for the code situation, i guess learned differently :)
Updated by Ben Touchette over 5 years ago
Wish i could fix it. Seems to work on my end :
cp test/data/exiv2-bug1199.webp 1199.webp ; cp test/data/Reagan.tiff . ; exiv2 -ea --force Reagan.tiff ; mv Reagan.exv 1199.exf ; exiv2 -pS 1199.webp ; exiv2 -ix 1199.webp ; exiv2 -pS 1199.webp
STRUCTURE OF WEBP FILE: 1199.webp
Chunk | Length | Offset | Payload
RIFF | 187526 | 0 | WEBP
VP8X | 10 | 12 | ,........
ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........
VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......]..
EXIF | 12040 | 172614 | II*........................... .
XMP | 2864 | 184662 | ./1199.exv: Failed to open the file
STRUCTURE OF WEBP FILE: 1199.webp
Chunk | Length | Offset | Payload
RIFF | 187526 | 0 | WEBP
VP8X | 10 | 12 | ,........
ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........
VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......]..
EXIF | 12040 | 172614 | II*........................... .
XMP | 2864 | 184662 |
i did an svn update, let me do a clean checkout and try again.
Updated by Ben Touchette over 5 years ago
Oops
cp test/data/exiv2-bug1199.webp 1199.webp ; cp test/data/Reagan.tiff . ; exiv2 -ea --force Reagan.tiff ; mv Reagan.exv 1199.exf ; exiv2 -pS 1199.webp ; exiv2 -ix 1199.webp ; exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp ./1199.exv: Failed to open the file STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp
Updated by Ben Touchette over 5 years ago
Ok now able to reproduce it, there was a typo in the command stack up there :)
Updated by Ben Touchette over 5 years ago
Fixed #4, turns out there was no padding for odd sized chunks for the exif and xmp data. Added a check and output the extra padding character needed for those cases.
Updated by Ben Touchette over 5 years ago
It also still breaks on -iX when i tried:
rm *.webp *.tiff *.exv *.xmp *.icc ; cp test/data/exiv2-bug1199.webp 1199.webp ; cp test/data/Reagan.tiff . ; exiv2 -eX --force Reagan.tiff ; mv Reagan.xmp 1199.xmp ; exiv2 -pS 1199.webp ; exiv2 -iX 1199.webp ; exiv2 -pS 1199.webp STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 187526 | 0 | WEBP VP8X | 10 | 12 | ,........ ICCP | 560 | 30 | ...0ADBE....mntrRGB XYZ ........ VP8 | 172008 | 598 | .G...*.. .>1..B.!..o.. ......].. EXIF | 12040 | 172614 | II*........................... . XMP | 2864 | 184662 | <?xpacket begin="..." id="W5M0Mp size 189288 size 189289 STRUCTURE OF WEBP FILE: 1199.webp Chunk | Length | Offset | Payload RIFF | 189289 | 0 | WEBP P8X | 738197504 | 13 | .........ICCP0......0ADBE....mnt
Updated by Ben Touchette over 5 years ago
The size is wrong in the RIFF header vs actual file size. Will investigate further in the morning
Updated by Robin Mills over 5 years ago
Loads of changes to deal with the padding byte. Please review: r4418
Comments:
1 TAG_SIZE
I've moved TAG_SIZE to include/exiv2/webpimage.hpp and initialised it in WebPImage::WebPImage(). Can you have a look at all those 4s in the code.
2 Height/Width
I'm puzzled by the code for decoding height and width. There are 4's and 7's in the code. 3 byte littleEndians or something. Can you put a comment in the code about this. Perhaps you should say something about it in the Wiki: http://dev.exiv2.org/projects/exiv2/wiki/The_Metadata_in_WEBP_files
3 Symbols for Bits in the header
I see there's some bit twiddling going on. I think you're setting flags to say "I've got an ICCP" or something like that. Can you give those values symbolic names using an enum in include/exiv2/webpimage.hpp.
4 Gotta be just about there now
a) Can you go over the code again, please. I suspect we can remove about 200 lines by refactoring repeated code into functions. For example when we write XMP and EXIF near line 365.
b) I'm also bothered about testing io_->eof() and io->size(). I'd like to see us navigate the file using file size from the RIFF header.
c) I personally never use 'break' except in switch/case. 'break' is a goto kind of thing.
5 What I'm going to do
a) Something broke yesterday in test/conversions.sh Groan. I'll investigate.
b) I think I have Gilles' multithreading issue off my back #1207
c) I'll deal with stdin/stdout such as -iX-
and the like. It's related to this, however it's a different matter.
Updated by Ben Touchette over 5 years ago
1- noticed no problemo
2- I have to decode it from the chunks available and reinsert when creating a new VP8X chunk, unavoidable
3- Yes i can do
4a- I can double check
4b- No it needs to be done because we need the new size from the actual data we write back.
4c- Yeah to break out of a while loop you need to break;
I noticed you removed a critical check at the end of the chunk reading code in writeMetaData and was in there because we need read and account for the damned padding character. Now it will break again on odd size chunks by not account for that byte.
Updated by Ben Touchette over 5 years ago
For the width & heights unfortunately they encode it into 28 bits of 32bit field so there's lots of bit shifting to get the actual sizes.
Updated by Ben Touchette over 5 years ago
Updated, fixed and commented, all the tests run 100%.
Updated by Robin Mills over 5 years ago
Some polishing - r4421. Please review. Apologies if I've accidentally broken your code.
I think we're now "done" on this. I believe we've successfully carried out our plan above: http://dev.exiv2.org/issues/1199#note-12
The code is beautiful. I'm very pleased indeed with what has been done. I'm sure we'll discover something in the weeks and months ahead. Thank You very much indeed. I'm going OpenHub to give you Kudos for a great job.
Updated by Robin Mills about 5 years ago
- Status changed from Assigned to Closed
- % Done changed from 80 to 100
Thanks very much for working on this, Ben. I has been a pleasure to work with you. I'm going to mark this "closed" as we head into feature complete on v0.26.
#1199 WebP Support (work in progress). Thank You to Ben for the patch. Thanks to Gilles for encouragement.