Conda-forge.github.io: 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 https://github.com/conda-forge/staged-recipes/pull/6842#issuecomment-436780495

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
https://github.com/conda-forge/clangdev-feedstock/pull/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" [e-mail] 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
https://github.com/conda-forge/conda-forge.github.io/issues/673#issuecomment-437341181,
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 https://github.com/conda-forge/libjpeg-turbo-feedstock/pull/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
https://github.com/conda-forge/conda-forge-pinning-feedstock/pull/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.

[hmaarrfk]

1. Pull jpeg8d
   
   
   
   • Hypothesis is that no packages depend on it
   
   
   • This was originally built as a stepping stone for this jpeg migration https://github.com/conda-forge/jpeg-feedstock/pull/20
   
   
2. Add a virtual package jpeg version 8d that depends on libjpeg-turbo
3. Migrate from jpeg 9 to jpeg 8.

[jschueller]

I think I finished https://github.com/conda-forge/libjpeg-turbo-feedstock/pull/15

[hmaarrfk]

Thanks.

I still wonder if it is cleaner to never provide a jpeg8 from an other source.

[isuruf]

@conda-forge/libjpeg-turbo, @conda-forge/jpeg, there are two options here.

1. Use libjpeg-turbo throughout, make it conflict with jpeg and deprecate jpeg.
2. Add a jpeg 8 virtual package from libjpeg-turbo, build with the original jpeg 8 and switch at install time like we do for BLAS.

I'm okay with either option.

[jschueller]

1. looks simpler but is it possible to issue a migration to replace a package with another ? If it isnt that means that we'll have to issue prs in many packages, so we might just as well use 2.

[isuruf]

looks simpler but is it possible to issue a migration to replace a package with another ?

Yes, https://github.com/regro/cf-scripts/blob/41941100ef98a4fc3a44eba3fb829c2faebaf492/conda_forge_tick/auto_tick.py#L632-L639

[jschueller]

Then what do you think on 1. guys ? I can try to implement a migrator.

[isuruf]

jpeg maintainers will have to weigh in on 1. @msarahan, @ocefpaf, @jakirkham, @gillins
Also defaults channel maintainers, @jjhelmus, @mingwandroid, @katietz, @chenghlee

[gillins]

1. Sounds great to me! :+1:

[jakirkham]

I think there are some use cases where people have needed jpeg version 9, which has functionality not in libjpeg-turbo. However my recollection here is pretty fuzzy (maybe @ocefpaf knows more?). So I would lean towards 2.

[ocefpaf]

I think there are some use cases where people have needed jpeg version 9, which has functionality not in libjpeg-turbo. However my recollection here is pretty fuzzy (maybe @ocefpaf knows more?). So I would lean towards 2.

That was true a while back. I have to confess I lost track of the old packages that needed it and I'm no longer supporting them. I guess that we should move forward and, as the exceptions emerge, we deal with them separately. There is no need to halt this for 1-2 old package out there.

[sdvillal]

I would also vote for 1).

Since turbo and jpeg 8 are ABI compatible, the ability to swap at runtime is, I think, something we do not need to care about, as it can be provided as an afterthought. To my mind it is unlikely anybody would want to go back to jpeg once they are using turbo.

In any case I believe we need a mutex package, because the alternative of having both turbo and libjpeg named the same and being chosen using the build string is not appealing to me (I do not know if conda has now some other mechanism to express something like "turbo provides+conflict with jpeg"). So even if we already have jpeg 8d packaged in conda-forge, 2) sounds like a bit more work for us.

Deprecating jpeg also sends a simpler message about our commitment to turbo.

In any case, we need for sure to rebuild all dependents and likely to avoid mixing turbo with libjpeg 9 in the same environment. And hopefully do it in coordination with defaults. I think https://github.com/conda-forge/libjpeg-turbo-feedstock/pull/15 plus a migrator goes in the right direction.

[sdvillal]

On a second thought, https://github.com/conda-forge/libjpeg-turbo-feedstock/pull/15 might not be what we want. Is feature tracking still a thing with conda? It used to bring a lot of headaches back in the day.

What are the alternatives we have to make turbo and jpeg conflict?

[isuruf]

What are the alternatives we have to make turbo and jpeg conflict?

run_constrained

[jakirkham]

Yeah run_constrained and a mutex package makes sense 🙂

[isuruf]

Both doesn't make sense. Either you have run_constrained that conflicts libpjpeg-turbo with jpeg or have a mutex package.