Switch to / provide libjpeg-turbo

[sdvillal]

TL;DR; libjpeg-turbo is faster and arguably more standard than jpeg 9, while it is not a drop-in replacement for jpeg 9. Should we consider switching back to jpeg 8 and making libjpeg-turbo conda forge default implementation?

Copy and paste from conda-forge/staged-recipes#6842 (comment)

conda-forge and defaults use jpeg 9. I think turbo is just unused (even if installed, it is not drop in because its library major is 8 and therefore the so files do not clobber - which would be warned about by conda itself). If any library uses turbo, that gets the user into segfaulty risk.

Using jpeg 9 is IMO unfortunate because turbo is becoming the de-facto standard everywhere else: from browsers to linux distros passing by downstream libraries like opencv or tensorflow use or will be using turbo. In fact libjpeg-turbo is currently under consideration for becoming an official ISO/ITU-T reference jpeg implementation. That is for good reason: turbo is an amazing implementation that runs about anywhere and is just much faster - shameless plug, see this blogpost of mine: http://blog.loopbio.com/video-io-2-jpeg-decoding.html.

Unfortunately, turbo is purposely not an ABI drop-in replacement for libjpeg 9, but for libjpeg 8 (see https://libjpeg-turbo.org/About/Jpeg-9). And that has given us a fair share of headaches in the past, to the extent that we went far to avoid symbol collision - but we thought our efforts were too hacky to try to push them into conda-forge. That is why you would see me being a bit conservative. Better safe than sorry.

I do not know about mixing turbo 8bit and 12bit in the same process, but I can only imagine that if used sloppily only the first version found by the dynamic loader will be used. Which I think at the moment would mean, at the very least, some surprises for the user intending to use 8bit, but getting 12bits, or vice-versa.

To my mind, the safest move is to depend on jpeg 9 like the rest of the stack. The right move, to my mind, should be to move defaults/conda-forge channels to use turbo. But I do not really know how hard that would be. Now, this is with my baggage. If you get to test this properly and are comfortable we are not getting our users into trouble by mixing several libraries (I do not know, maybe playing with symbol visibility/versioning, or just assuming that no trouble will happen), go ahead and provide that speedup, that should always be welcome.

A small read: http://www.fcollyer.com/2013/01/04/elf-symbol-visibility-and-the-perils-of-name-clashing/
Our renaming hack to avoid symbol collision: https://github.com/loopbio/libjpeg-turbo-feedstock
A test in our opencv build: https://github.com/loopbio/opencv-feedstock/blob/loopbio/recipe/ensure_jpegturbo_opencv_plays_nicely_with_jpeg9.py

[ocefpaf]

I'm in favor of this change. Maybe we can out up a migrator, ping @CJ-Wright, to implement this change. I believe the first step would be check its ABI and add a pin to the conda-forge-pinning package.

[CJ-Wright]

I think pinning would be a first good step. It could be possible to make a migrator for this. Another good first step would be to find the scope of the migration (how many things use 'jpeg').

[CJ-Wright]

@conda-forge/bot

[CJ-Wright]

@sdvillal any interest in putting a PR into regro/cf-scripts for a new migrator? I'm happy to walk you through the process/review PRs.

[sdvillal]

I think this change should be made in coordination with defaults, otherwise we risk introducing quite a sneaky incompatibility between both stacks. I also do not know if any downstream dependency do require jpeg 9 (@jakirkham seemed to have some vague memories for this).

@CJ-Wright yeah, I can do that. I'm a bit short of bandwidth until around mid next-week, I will likely start then.

@ocefpaf from https://abi-laboratory.pro/?view=timeline&l=libjpeg-turbo I would pin to x.x (there was a soname change between 1.2 and 1.3).

[sdvillal]

Actually maybe we should keep the pin to x (ABI seems really stable and that soname change is more of a false positive).

[sdvillal]

Since with conda we cannot say "libjpeg-turbo provides jpeg", I think building jpeg packages for versions (6, 8) that actually just install turbo is our safest bet to avoid parallel installs.

[ocefpaf]

Sound reasonable to me. Thanks for looking into this @sdvillal!

[msarahan]

I think the best way to handle this is with a mutex metapackage. Comma forge recently went through that for clangdev. I need to write up some docs on what the important parts of that are. For now, please refer to the PR at conda-forge/clangdev-feedstock#40 There's more discussion on gitter between Sylvain and I, if you scroll up a few days. On Nov 9, 2018 06:08, "Filipe" <[email protected]> wrote: Sound reasonable to me. Thanks for looking into this @sdvillal <https://github.com/sdvillal>! — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#673 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACV-aGte7FdGOW3jK5nQLMGQek8_wJ4ks5utXAggaJpZM4YTbOp> .

[jschueller]

hi @msarahan, any updates on this ?

[isuruf]

@jschueller, please send a PR to libjpeg-turbo-feedstock to add a new output jpeg with version 8d and depends on libjpeg-turbo. Also add build: track_features: jpeg_impl_libjpeg_turbo to make it not the default.

[jschueller]

see conda-forge/libjpeg-turbo-feedstock#15

[jschueller]

I see, so then we can pin jpeg to 8d instead of 9 in the whole stack, then we can make the switch, right ?

[jakirkham]

@jschueller, I think people have already been doing work on this. Would talk to @hmaarrfk who I believe was leading this effort.

[hmaarrfk]

I made a PR to the migrator to start to build out jpeg 8d again. There was some discussion about whether or not we should build both ajpeg 8 and 9

[jschueller]

I'm curious to know about the v9 motivations

[hmaarrfk]

For me it is just that precedent is hard to change

Follow progress here
conda-forge/conda-forge-pinning-feedstock#378

[hmaarrfk]

Could we use a name migrator and just skip the whole abi compatibility constraint?

I'm trying to find the matplotlib -> matplotlib-base migrator but I can't seem to find it.

Would something like that be acceptable?

Since turbo is so widely used, I don't anticipate much breakage we won't be able to address.

[isuruf]

Nope. You would then end up with some packages linking to libjpeg-turbo and some linking to jpeg.

[hmaarrfk]

So we would have to make that exclusive package or libjpeg-turbo and jpeg before we start the migration right?

[hmaarrfk]

during migrations, there are always some packages built against the "old version" while some packages are built with the "new version" right?

[hmaarrfk]

@isuruf this seems to be above my level skills ensuring things stay backward compatible.

I might simply suggest cutting out losses and:

1. Pulling the jpeg8 package from conda-forge
2. Making jpeg8 synonymous with libjpeg-turbo

calling things a day.

Defaults doesn't provide jpeg8 so that might solve the compatibility issue.

[traversaro]

Given that conda-forge/conda-forge-pinning-feedstock#4945 was merged, perhaps we can close this issue? Or there something else to fix?