support for ZSTD compression for TIFF files #2273

kindoblue · 2020-09-14T10:23:39Z

Why we need this PR?

It adds ZSTD compression handling for TIFF images
explicitly builds libz and ships it with DALI wheel

What happened in this PR?

What solution was applied:
builds libzstd and libz and adds it as a wheel dependency
Affected modules and functionalities:
ImageDecoder operator, in case a TIFF file is opened
Key points relevant for the review:
N/A
Validation and testing:
currente tests with new test TIFF file added in Adds TIFF image with ZSTD encoding DALI_extra#46
Documentation (including examples):
N/A

JIRA TASK: NA

JanuszL · 2020-09-14T11:01:34Z

@kindoblue - thank you for your contribution.
I have a couple of remarks:

can you build libzstd from sources in the deps file? I think it would be safer and better to have the latest version available in DALI while the system one could be pretty outdated (and may be sensitive to security issues)
can you add this to Dockerfile.build.aarch64-qnx and Dockerfile.build.aarch64-linux as well (we don't publish binaries for this architectures but we are maintaining internal build for it)
can you add an appropriate set of test files to our test data repository - https://github.com/NVIDIA/DALI_extra/tree/master/db/single/tiff (please follow the contribution guide or tell us how to generate files in the relevant format so we can add it on our own)?
please sign off your commit

kindoblue · 2020-09-15T15:32:25Z

To generate tiff files compressed with ZSTD I used the python library tifffile

For example you can do:

    img_a = cv2.imread("lena_std.tif", cv2.IMREAD_COLOR)
    im_rgb = cv2.cvtColor(img_a, cv2.COLOR_BGR2RGB)
    tifffile.imwrite("lena_std.zstd.tif", im_rgb, compress='ZSTD')

I actually use tifffile to create multichannel TIFFs from NumPy arrays.

JanuszL · 2020-09-15T17:14:02Z

To generate tiff files compressed with ZSTD I used the python library tifffile

For example you can do:
    img_a = cv2.imread("lena_std.tif", cv2.IMREAD_COLOR)
    im_rgb = cv2.cvtColor(img_a, cv2.COLOR_BGR2RGB)
    tifffile.imwrite("lena_std.zstd.tif", im_rgb, compress='ZSTD')
I actually use tifffile to create multichannel TIFFs from NumPy arrays.

Thanks! I will create some additional test data soon.

JanuszL · 2020-09-16T10:24:40Z

@kindoblue - can you check if NVIDIA/DALI_extra#46 does what you need?

kindoblue · 2020-09-16T12:20:02Z

The RB channels in the picture are swapped. The bps value reported by the file command is strange but anyway geeqie opens the file. Later today I will check better.

JanuszL · 2020-09-16T12:37:13Z

@kindoblue - I have fixed the channels. Should be fine now.

kindoblue · 2020-09-16T19:15:40Z

I can create a pipeline and load the picture.

I am taking a look at the gtest suite but even if I run those tests, dali and the operators will run on the deps I have installed on my machine, not from the packaged wheel, right? But anyway, regarding that particular TIFF file, I tested it manually.

JanuszL · 2020-09-17T08:20:33Z

docker/Dockerfile.build.aarch64-qnx

+RUN ZSTANDARD_VERSION=1.4.5 && \
+    curl -L https://github.com/facebook/zstd/releases/download/v${ZSTANDARD_VERSION}/zstd-${ZSTANDARD_VERSION}.tar.gz | tar -xzf - && \
+    cd zstd-${ZSTANDARD_VERSION} && \
+    make -j"$(grep ^processor /proc/cpuinfo | wc -l)" install 2>&1 >/dev/null && \


You need to cross compile it for aarch64, see how other libraries are build.

I was taking a look at QNX, I didn't even manage to understand which toolchain is good for Linux. Any pointer? I did some programming on embedded platforms in the past but never with QNX.

I guess it should be something like this:

CFLAGS="-fPIC" \ CXXFLAGS="-fPIC" \ CC=aarch64-unknown-nto-qnx7.0.0-gcc \ CXX=aarch64-unknown-nto-qnx7.0.0-g+ \ prefix=/usr/aarch64-unknown-nto-qnx/aarch64l \ make install -j"$(grep ^processor /proc/cpuinfo | wc -l)" && \

And similar (check how other deps are build there) for aarch64.

And regarding the aarch64 for Nvidia Jetson, the SDKManager doesn't allow the download for ubuntu 20.04 :-(

Sorry, I cannot do much about it.

I'm trying to get a temp license for qnx and test the compilation on that platform. Regarding the aarch64 for nvidia jetson, I will see in the coming days to use a box with ubuntu 18.04 installed

Great!.
Thanks.

So I managed to install on my host (ubuntu 20.04) qnx 7.1 and I tried to compile the zstd library. As I cannot use the SDKManager I could not test the docker build but I think the modifications I did in the last commits should work. On my host the zstd library is correctly compiled and linked

lib file libzstd.so.1.4.6 libzstd.so.1.4.6: ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV), dynamically linked, BuildID[md5/uuid]=e96f8cf465e8cec436c1487611392dc9, with debug_info, not stripped

Sure, let me try it on our CI.

JanuszL · 2020-09-17T08:20:38Z

docker/Dockerfile.deps

+# zstandard compression library
+RUN ZSTANDARD_VERSION=1.4.5 && \
+    curl -L https://github.com/facebook/zstd/releases/download/v${ZSTANDARD_VERSION}/zstd-${ZSTANDARD_VERSION}.tar.gz | tar -xzf - && \
+    cd zstd-${ZSTANDARD_VERSION} && \


You need to cross compile it for aarch64, see how other libraries are build.

JanuszL · 2020-09-17T08:22:55Z

I am taking a look at the gtest suite but even if I run those tests, dali and the operators will run on the deps I have installed on my machine, not from the packaged wheel, right? But anyway, regarding that particular TIFF file, I tested it manually.

Please build the DALI wheel - https://docs.nvidia.com/deeplearning/dali/user-guide/docs/compilation.html#compiling-dali-from-source-using-docker-builder-recommended, install it on your host system (the wheel uses own, not system one dependencies) and then run the test from the place where the wheel has been installed python -c 'import os; from nvidia import dali; print(os.path.dirname(dali.__file__))'/test

kindoblue · 2020-09-18T14:53:18Z

I managed to run the tests withing the virtualenv, after having compiled the wheel. I used your DALI_extra branch:

➜  DALI_extra git:(more_tifs) ✗ git status
On branch more_tifs
Your branch is up to date with 'origin/more_tifs'.

These are the test results (not sure if the ZSTD tiff would be automatically included in the tests):

(deep) ➜  test ./dali_core_test.bin

[==========] 269 tests from 50 test suites ran. (2955 ms total)
[  PASSED  ] 269 tests.

  YOU HAVE 4 DISABLED TESTS



(deep) ➜  test ./dali_test.bin

[==========] 358 tests from 80 test suites ran. (14851 ms total)
[  PASSED  ] 356 tests.
[  SKIPPED ] 2 tests, listed below:
[  SKIPPED ] CApiTest/0.ExternalSourceSingleAllocDifferentBackendsTest
[  SKIPPED ] CApiTest/0.ExternalSourceMultipleAllocDifferentBackendsTest

  YOU HAVE 5 DISABLED TESTS


(deep) ➜  test ./dali_kernel_test.bin

[==========] 1594 tests from 412 test suites ran. (68384 ms total)
[  PASSED  ] 1594 tests.

  YOU HAVE 2 DISABLED TESTS


(deep) ➜  test ./dali_operator_test.bin

[==========] 4753 tests from 222 test suites ran. (312595 ms total)
[  PASSED  ] 4748 tests.
[  SKIPPED ] 1 test, listed below:
[  SKIPPED ] HwDecoderUtilizationTest.UtilizationTest
[  FAILED  ] 4 tests, listed below:
[  FAILED  ] VideoReaderTest.FrameLabels
[  FAILED  ] VideoReaderTest.FrameLabelsWithFileListFrameNum
[  FAILED  ] VideoReaderTest.TimestampLabels
[  FAILED  ] VideoReaderTest.StartEndLabels

 4 FAILED TESTS
  YOU HAVE 4 DISABLED TESTS

JanuszL · 2020-09-18T20:30:07Z

Test results looks good, please update aarch and qnx deps dockers, rebase your PR and update DALI_EXTRA_VESRION content to ad36cfb37f1c9bde2062051520698d0ac0328962 (it is sha of the merge commit with new test TIF).
Then I will launch our CI.

Signed-off-by: Stefano <kindoblue@gmail.com>

dali-automaton · 2020-09-21T10:48:07Z

CI MESSAGE: [1641658]: BUILD STARTED

dali-automaton · 2020-09-21T11:04:09Z

CI MESSAGE: [1641688]: BUILD STARTED

dali-automaton · 2020-09-21T12:28:39Z

CI MESSAGE: [1641786]: BUILD STARTED

JanuszL · 2020-09-21T13:33:38Z

docker/Dockerfile.build.aarch64-linux

+    curl -L https://github.com/facebook/zstd/releases/download/v${ZSTANDARD_VERSION}/zstd-${ZSTANDARD_VERSION}.tar.gz | tar -xzf - && \
+    cd zstd-${ZSTANDARD_VERSION} && \
+    CFLAGS="-fPIC" CXXFLAGS="-fPIC" CC=aarch64-linux-gnu-gcc CXX=aarch64-linux-gnu-g++ \
+      make -j"$(grep ^processor /proc/cpuinfo | wc -l)" install DESTDIR=/usr/aarch64-linux-gnu/ 2>&1 >/dev/null && \


It would be better to use prefix instead of DESTDIR as now it would install in
/usr/aarch64-linux-gnu/usr/local/....
I have also noticed that tiff build cmd needs to be updated, CFLAGS and CXXFLAGS need to be set to "-fPIC -I/usr/aarch64-unknown-nto-qnx/aarch64le/include" to include a new dependency.

You are right, DESTDIR and prefix have different behaviour. Regarding the flag for the tiff library, I am afraid you need to use the configure flag --with-zstd-include-dir and --with-zstd-lib-dir and not the -I because that, I tested on my host, doesn't work. Anyway I will make the modification later on
Or perhaps, once changed from DESTDIR to prefix, the zstd will be found because now the prefix dir would be correct.

I did the modification only for the prefix. I think it should work then. Sorry for that, but I am working in the dark :-)

Sure, no problem. We really appreciate your time and patience.

Or perhaps, once changed from DESTDIR to prefix, the zstd will be found because now the prefix dir would be correct.

I think it won't work just like that as configure searches the default system directories, not the one we use in the prefix.

Checked on my host. You are again right. I will use the --with-zstd-- flags

dali-automaton · 2020-09-21T14:16:04Z

CI MESSAGE: [1641786]: BUILD FAILED

Signed-off-by: Stefano <kindoblue@gmail.com>

dali-automaton · 2020-09-21T15:35:27Z

CI MESSAGE: [1641688]: BUILD FAILED

dali-automaton · 2020-09-22T10:23:49Z

CI MESSAGE: [1644611]: BUILD STARTED

dali-automaton · 2020-09-22T11:53:35Z

CI MESSAGE: [1644611]: BUILD FAILED

dali-automaton · 2020-09-22T12:00:18Z

CI MESSAGE: [1644772]: BUILD STARTED

dali-automaton · 2020-09-23T10:28:29Z

CI MESSAGE: [1647820]: BUILD STARTED

dali-automaton · 2020-09-23T10:41:21Z

CI MESSAGE: [1647820]: BUILD FAILED

dali-automaton · 2020-09-23T11:04:43Z

CI MESSAGE: [1647899]: BUILD STARTED

dali-automaton · 2020-09-23T11:28:06Z

CI MESSAGE: [1647899]: BUILD FAILED

dali-automaton · 2020-09-23T11:55:46Z

CI MESSAGE: [1647951]: BUILD STARTED

dali-automaton · 2020-09-23T12:16:28Z

CI MESSAGE: [1647951]: BUILD FAILED

dali-automaton · 2020-09-23T13:11:52Z

CI MESSAGE: [1648052]: BUILD STARTED

dali-automaton · 2020-09-23T15:52:33Z

CI MESSAGE: [1648052]: BUILD PASSED

klecki · 2020-09-23T16:18:21Z

docker/Dockerfile.deps

+    cd /tmp && \
+    curl -L https://github.com/madler/zlib/archive/v${LIBZ_VERSION}.tar.gz | tar -xzf - && \
+    cd zlib-${LIBZ_VERSION} && \
+    ./configure --prefix=/usr/local && \


Don't we need PIC here as well?

dali-automaton · 2020-09-23T20:29:04Z

CI MESSAGE: [1649140]: BUILD STARTED

dali-automaton · 2020-09-23T20:40:48Z

CI MESSAGE: [1649140]: BUILD FAILED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

dali-automaton · 2020-09-23T20:48:28Z

CI MESSAGE: [1649201]: BUILD STARTED

dali-automaton · 2020-09-23T23:26:45Z

CI MESSAGE: [1649201]: BUILD FAILED

dali-automaton · 2020-09-24T07:47:26Z

CI MESSAGE: [1649201]: BUILD PASSED

jantonguirao · 2020-09-24T08:37:40Z

Acknowledgements.txt

@@ -2643,3 +2643,70 @@ INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
 CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
 ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 POSSIBILITY OF SUCH DAMAGE.
+
+========================
+


I think we should say here that this is for zstd. @JanuszL ?

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

dali-automaton · 2020-09-24T09:33:38Z

CI MESSAGE: [1650977]: BUILD STARTED

dali-automaton · 2020-09-24T12:09:30Z

CI MESSAGE: [1650977]: BUILD PASSED

JanuszL · 2020-09-24T12:14:06Z

@kindoblue - thank you for your contribution!
This change should be available in tomorrow nightly build.

kindoblue · 2020-09-24T12:18:29Z

Thanks to you guys!

kindoblue force-pushed the ZSTD_compression_support branch from db0459b to b87d76c Compare September 15, 2020 14:00

JanuszL reviewed Sep 17, 2020

View reviewed changes

support for ZSTD compression for TIFF files

88a2e66

Signed-off-by: Stefano <kindoblue@gmail.com>

kindoblue force-pushed the ZSTD_compression_support branch from b87d76c to 4edde4e Compare September 19, 2020 08:49

DALI_EXTRA_VERSION updated

03ac616

Signed-off-by: Stefano <kindoblue@gmail.com>

kindoblue force-pushed the ZSTD_compression_support branch from 4edde4e to 03ac616 Compare September 19, 2020 08:49

kindoblue force-pushed the ZSTD_compression_support branch from 5135ee3 to b0816a3 Compare September 21, 2020 12:30

JanuszL reviewed Sep 21, 2020

View reviewed changes

kindoblue force-pushed the ZSTD_compression_support branch from b0816a3 to d11d42b Compare September 21, 2020 14:21

compilation flags for aarch64 (linux and qnx) should be now fixed

3be9658

Signed-off-by: Stefano <kindoblue@gmail.com>

kindoblue force-pushed the ZSTD_compression_support branch from d11d42b to 3be9658 Compare September 21, 2020 14:53

JanuszL force-pushed the ZSTD_compression_support branch 2 times, most recently from 324f9fc to d2d1904 Compare September 23, 2020 11:54

JanuszL force-pushed the ZSTD_compression_support branch from d2d1904 to c090563 Compare September 23, 2020 13:09

JanuszL approved these changes Sep 23, 2020

View reviewed changes

klecki reviewed Sep 23, 2020

View reviewed changes

JanuszL force-pushed the ZSTD_compression_support branch from c090563 to 5256982 Compare September 23, 2020 20:19

Fix

4277b6e

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

JanuszL force-pushed the ZSTD_compression_support branch from 5256982 to 4277b6e Compare September 23, 2020 20:45

jantonguirao approved these changes Sep 24, 2020

View reviewed changes

Review fix

36366b4

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

jantonguirao approved these changes Sep 24, 2020

View reviewed changes

klecki approved these changes Sep 24, 2020

View reviewed changes

JanuszL merged commit b821084 into NVIDIA:master Sep 24, 2020

support for ZSTD compression for TIFF files #2273

support for ZSTD compression for TIFF files #2273

Conversation

kindoblue commented Sep 14, 2020 • edited by JanuszL Loading

Why we need this PR?

What happened in this PR?

JanuszL commented Sep 14, 2020

kindoblue commented Sep 15, 2020

JanuszL commented Sep 15, 2020

JanuszL commented Sep 16, 2020

kindoblue commented Sep 16, 2020

JanuszL commented Sep 16, 2020

kindoblue commented Sep 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JanuszL commented Sep 17, 2020

kindoblue commented Sep 18, 2020

JanuszL commented Sep 18, 2020

dali-automaton commented Sep 21, 2020

dali-automaton commented Sep 21, 2020

dali-automaton commented Sep 21, 2020

Choose a reason for hiding this comment

kindoblue Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

kindoblue Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kindoblue Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

dali-automaton commented Sep 21, 2020

dali-automaton commented Sep 21, 2020

dali-automaton commented Sep 22, 2020

dali-automaton commented Sep 22, 2020

dali-automaton commented Sep 22, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 23, 2020

dali-automaton commented Sep 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dali-automaton commented Sep 24, 2020

dali-automaton commented Sep 24, 2020

JanuszL commented Sep 24, 2020 • edited Loading

kindoblue commented Sep 24, 2020

kindoblue commented Sep 14, 2020 •

edited by JanuszL

Loading

kindoblue commented Sep 16, 2020 •

edited

Loading

kindoblue Sep 21, 2020 •

edited

Loading

kindoblue Sep 21, 2020 •

edited

Loading

kindoblue Sep 21, 2020 •

edited

Loading

JanuszL commented Sep 24, 2020 •

edited

Loading