Review Request 113397: RAW preview in gwenview

[Martin Kyral]

> On Oct. 24, 2013, 8:17 a.m., Aurélien Gâteau wrote:
> > I am sorry but support for remote urls is a core feature of Gwenview, it cannot be ignored for Raw images. I had a look at LibRaw API, it supports reading images from a buffer (see http://www.libraw.org/docs/API-CXX-eng.html#open_buffer ), so I suggest either extending KDCraw API to support this or bypassing KDCraw and use LibRaw directly.
> > 
> > Unfortunately, the changes you made to the buffer handling does not prevent loading the data twice: mData already contains the image data by the time the code is called.
> 
> Martin Kyral wrote:
>     That's a little complication, then. However, thanks for the hint. I have looked how KDcraw::loadEmbeddedPreview is implemented and it uses LibRaw. So, it won't be hard to make such change.
>     There are 3 possible ways:
>     1) implement it in KDcraw
>        pros: clean solution, fixed upstream, other KDE projects can benefit from the change
>        cons: would delay the feature in gwenview, will take some time as it needs reimplementation of the function (implement new one reading from the buffer and changing the original so it  opens a file in the buffer and calls the new one) and it will likely cause a need for the same chane in the other KDcraw::load* functions.
>     2) use LibRaw directly
>        pros: quick, no delay
>        cons: pros of 1) inverted
>     3) wrap this part of LibRaw functionality in a separate function having the same interface as the function proposed for KDcraw and use it until the KDcraw change is ready.
>        pros: combines pros of 1) and 2), the raw preview unpacking shall go in a function anyway as it is used twice in the code now
>        cons: future action needed (switch to KDcraw)
>     
>     I'd vote for 3). How do you feel about it?
> 
> Aurélien Gâteau wrote:
>     You might want to check if you can't get solution 1) done: KDCraw is part of KDE SC, so there is no need to worry about Gwenview building with an older version of the library. If it's not possible, then solution 3) sounds good.

I already implemented 3) as a POC and it works fine so people can test it :)
Now I am looking in KDcraw. The function in KDcraw shall be exactly like the one in this patch. The current KDcraw::loadFormData(QByteArray, QString) shall be modified so it only loads the file and passes its contents via QBuffer to prevent duplication of the code.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113397/#review42263
-----------------------------------------------------------


On Oct. 25, 2013, 8:07 a.m., Martin Kyral wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113397/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2013, 8:07 a.m.)
> 
> 
> Review request for Gwenview and Aurélien Gâteau.
> 
> 
> Repository: gwenview
> 
> 
> Description
> -------
> 
> This is a review of patch enabling raw support in gwenview. It uses KDcraw to extract the embedded jpeg preview (most of the raw formats contain it for the purposes of quick viewing the photos on the camera display) so it is fast, implementing was quite easy and theoretically it enables gwenview to support everything dcraw supports (practically we need to test more formats). The patch does not perform demosaicing and I don't think it shall - digikam, darktable or rawtherapee are much more suited for developing the 'digital negatives'.
> 
> The patch fixes BZ#158788
> 
> The patch enables support for Nikon NEF and NRW formats, Canon's CR2, Pentax PEF, Sony ARW and Adobe DNG. If you want to try another format, just adding it's MIME type to the list in lib/mimetypeutils.cpp shall be enough.
> 
> The patch as is contains just the basic functionality (generating proper thumbnails and viewing the raw images incl. EXIF metadata), but it is well usable. However, there is still a lot of stuff to do (see the commit message in the patch) so I'd appreciate any help.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 4dafb7e 
>   lib/CMakeLists.txt c6ffe14 
>   lib/document/loadingdocumentimpl.cpp fbad8ff 
>   lib/glibraw.h PRE-CREATION 
>   lib/glibraw.cpp PRE-CREATION 
>   lib/gwenviewconfig.kcfg 231fd5e 
>   lib/mimetypeutils.cpp 546346f 
>   lib/thumbnailprovider/thumbnailgenerator.cpp 8b98904 
> 
> Diff: http://git.reviewboard.kde.org/r/113397/diff/
> 
> 
> Testing
> -------
> 
> Tested on some sample raw files taken from the camera reviews on http://www.photographyblog.com/ and bunch of my own photos. Nikon (tried D50, D70s, D90, D800 and Coolpix P7000), Canon (tried 7D, 5D Mark3 and 6D) and Pentax (K-X, K30) have the preview full-res, while Sony raw files (A900, A850, A58, Nex 3, Nex 6) have only 1616x1050.
> 
> 
> Thanks,
> 
> Martin Kyral
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gwenview-devel/attachments/20131025/00fd8d30/attachment.html>