[Scummvm-devel] [scummvm] Replace our JPEG decoder with one capable of handling progressive JPEGs (#374)

[A. Milburn]

Hi,

Context for those who haven't been following the JPEG discussion:
https://github.com/scummvm/scummvm/pull/374

(Short story: sev suggested some alternative JPEG decoding code, to add
support for progressive images, without which some Wintermute games don't
work; I grumped about it, everyone else grumped about it too.)


On Sat, Aug 10, 2013 at 05:40:00PM -0700, digitall wrote:
> However, libjpeg-turbo though having a better website and documentation
> than libjpeg, which is very terse, has a build system requiring CMake

It should be ABI-compatible (by default) with the old trusted standard
libjpeg6b/62 which pretty much everything still ships, so the theory is
that a port should be able to pick whichever is most convenient for them.

> and is mainly aimed at desktop systems i.e. it mainly adds support for
> SIMD type acceleration for desktop systems, which is not our problem.
> Our problem is slow performance on the lower end targets...

Well, it has SIMD code for x86/amd64 and NEON, and it can be twice as
fast as libjpeg. If we care about performance (and maybe battery life
although I doubt we decode JPEGs that much) on the modern mobile devices
then it might be worth considering.

> Overall, I think @sev- 's original code is a much better idea... but maybe

So I'm a bit dubious about the source because it's just some "public
domain" declaration without any permission statement like "do whatever you
want"; it's not clear that this is legally meaningful in many places, which
is why licenses like CC0 (aka the Public Domain Dedication) exist. The
author switched another "public domain" project to http://unlicense.org/
last year, so I expect we could just ask them to do the same for this code.

Otherwise.. maybe one important question is, how much do we care about
binary size? And how much do we care about importing this code without
applying coding-style etc, which is likely to just be a pain?

There are a lot of comments I could make on the contents of the code (e.g.
I guess the contents of the jpgd namespace could all go into a private
header file, which would not pollute our normal namespace with this) but
that's all "minor" compared to this larger discussion I guess.

> using this as a reference to improve/correct and add support for the
> progressive JPEG decoder to our existing decoder or a modification of this
> would be the best option. I assume that @scott-t will do this in time, if
> this is needed for ROQ/11H support.... or one of the Wintermute developers
> will look at this if needed.

According to the pull request it's not needed for GroovieV2. It's completely
blocking support for some Wintermute games and that's what sev wanted to fix
here, so I'm not sure leaving it to "the Wintermute developers" is such a
great plan; this pull request was intended to deal with that case, and it
was neatly shot down in flames.


In the end: the current code doesn't work for many standard images and it's
blocking support for games, and I think asking engine maintainers to deal
with a problem which was caused by us not just using libjpeg in the first
place is rather unfair.

If someone (whether ST or whoever) is volunteering to fix the existing code
(and without making the memory/CPU requirements worse for things like
Mohawk): great.

But otherwise I think we should look into a memory/performance comparison of
the other options, at least.

- Alyssa