Adds ability to enable Adobe DNG SDK #39
Conversation
|
Thanks for the effort on this. I think from a maintenance point of view I'm not super happy including third-party build files in this repository. Could I ask what your use case is? Quoting from https://github.com/LibRaw/LibRaw/blob/master/README.DNGSDK.txt:
|
So for unrelated reasons, I tweaked it so the DNG build script is now part of this Cmakefile which should make it easier to maintain and had benefit of solving some linker errors. So that's now resolved. There are some limitations with this script currently but hopefully we can resolve them (Its built for Linux and there are some issues regarding symbols which I have to tell linker to ignore) The XMP SDK is significantly more complicated since it's a series of targets each with their own various Cmakelists but considering it's available on Github and has scripts for Linux maintained by Adobe, it should be possible to just download the code and point this script to it's location similar to Rawspeed. I think doing this way is easier to maintain whilst reducing the learning curve but if this is still too much, perhaps I can instead allow user provided path to the library files emitted by the XMP toolkit when built. As well as provide instructions on how to compile it. The Use CaseSo our use-case is for DNGs which contain compressed JPEG data. Although the documentation claims that they are rare, which relative to the large amount of RAW files may be true, these kind of files can be exported by Adobe Lightroom by Photographers exporting an express DNG to reduce file sizes. Once you get to handling a certain volume DNG files, these kinds of files actually become very common. Unfortunately the stage2 operations needed aren't implemented by libraw yet and can only be done via Adobe's DNG SDK. The ProblemThe problem is although the libraw documentation mentions that in order to enable the Adobe SDK all you need is to set the USE_DNG flag to true and pass it an instance of the DNG SDK. It misses the steps on how to download and compile the SDK. Which is made more complicated that the SDK doesn't have any scripts for Linux. On top of this the cmake script and rawpy (PR coming soon) lacked the enabling of Adobe's DNG SDK support So in practice switching on this feature had a steep curve, which this PR hopes to reduce to just enabling the DNG SDK in the cmake script and pointing it to where the downloaded source code is. |
| add_definitions(-DqLinux) | ||
| add_definitions(-DUNIX_ENV=1) |
There was a problem hiding this comment.
The CMake files support Windows and macOS as well. Why would this feature be restricted to Linux?
There was a problem hiding this comment.
The CMake files support Windows and macOS as well. Why would this feature be restricted to Linux?
I just need to extend this code to detect the OS the system is building under, I'm not sure what the best way is to do this.
| raw | ||
| PUBLIC | ||
| # zlib | ||
| ${XMP_LIB_DIR}/staticXMPCore.ar |
There was a problem hiding this comment.
Since XMP is pulled in via add_subdirectory, doesn't it have CMake targets that could be referenced here?
There was a problem hiding this comment.
Since XMP is pulled in via
add_subdirectory, doesn't it have CMake targets that could be referenced here?
It does but what seems to be happening is in the Adobe build script it moves the .a static libraries to being a .ac file in a way that Cmake isn't aware of and if you attempt to link it using the targets then it will try to link to the .a files which no longer exist.
I'm not sure really what the solution was other than to point to it directly like this.
| @@ -633,6 +778,17 @@ if (LIBRAW_INSTALL) | |||
| install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules/FindLibRaw.cmake | |||
| DESTINATION ${CMAKE_INSTALL_DATADIR}/cmake/libraw) | |||
|
|
|||
| # Install DNG SDK's host header as downstream clients may need it | |||
There was a problem hiding this comment.
Is this standard practice for the DNG SDK? Normally, a library should only ship its own headers.
There was a problem hiding this comment.
Is this standard practice for the DNG SDK? Normally, a library should only ship its own headers.
No, I think we would have to move this to an install step for the DNG SDK. The reason being is downstream libraries need the "dng_host.h" in order to configure libraw to use the Adobe SDK as you need to pass it an instance of the dng host class.
Could you outline what this would look like? That would be my preferred option. It might be better to create a separate repo that allows to easily build the SDK, assuming this can't be upstreamed. |
Sp we would just need a location of the folder where the static libraries are compiled i.e Another option might just to provide the ability to provide a path to the compiled DNG SDK with XMP static libraries already linked i.e Another option rather than parsing the path could be to use pkg_config to detect an installed copy of the DNG SDK ? |
|
@jcampbell05 How do you want to continue here? I think the best would be to keep this repo as small as possible to reduce ongoing maintenance effort. |
|
I'll have to check if possible - but seems like best approach is just to have the lines to at least allow those who have gone through the effort of building the DNG and XMP SDK a way of telling the cmake project to compile with Adobe DNG support and where to find the libraries. This feels like a happy medium between keeping maintenance low but allowing those who need it to plug-and-play without having to dig through the cmake file. |
This makes changes to allow us to specify we want the Adobe SDK, it currently relies on a Cmake script I have in a fork of the DNG SDK (See here https://github.com/Autoenhance-ai/adobe-dng-sdk/blob/1.6-linux/dng_sdk/projects/linux/CMakeLists.txt), we could pull that script into this PR but would love your feedback first.