[quake3-bugzilla] [Bug 4564] Use system libjpeg instead of bundled copy (optionally)

[bugzilla-daemon at icculus.org]

https://bugzilla.icculus.org/show_bug.cgi?id=4564

--- Comment #7 from Thilo Schulz <arny at ats.s.bawue.de> 2011-02-10 14:32:33 EST ---
(In reply to comment #6)
> I believe the intention here is that the symbols in the executable will
> override the ones in libjpeg, causing its memory allocator/deallocator to be
> substituted. I'm not sure whether that actually happens, mind... the other
> possibility is that the ones in jmemnobs.c are ignored and we use malloc/free,
> which is harmless as long as we're consistent about it.

Well, I'd rather have it that system libjpeg uses system malloc/free. In this
case we don't need to enforce using Z_Malloc/Z_Free under all circumstances,
and can get rid of jmemnobs.c entirely.

> It's called by tr_image_jpeg, rather than from within libjpeg. One of the Q3
> changes was to replace the standard jpeg_stdio_src with an essentially
> equivalent jpeg_mem_src, but when we're using the system libjpeg, we can't rely
> on it having that modification, and we still need to supply our own
> jpeg_mem_src.

Yes! that's reasonable.

> The cleanest way to do this would probably be one of:
> 
> A. revert the changes to Q3's jdatasrc.c, and have the renderer always use the
> same jpeg_mem_src that we'd use with the system libjpeg

This is the way to go, I guess.

> However, libjpeg6b is part of the LSB, libjpeg8 isn't binary-compatible, and
> I'm not sure how much will catch fire if we're linked against libjpeg8 but our
> copy of SDL or whatever is linked against libjpeg6b. Debian started a
> transition to libjpeg8 a few months ago, but then reverted it all because of
> that sort of problem.

Then I would recommend building ioquake3 with system jpeglib enabled only for
distributors, where distributors can be sure they have the required
dependencies fulfilled. That sounds reasonable.

> > 3. Why the extra c-file jpeg_memsrc for the jpeg_mem_src function? Can't we
> > just put all into tr_image_jpeg.c?
> 
> Conceptually, it's an independent piece of utility code for libjpeg, rather
> than part of the Q3 renderer. As I said, in libjpeg 8 it's been incorporated
> into libjpeg.

Please put that function into tr_image_jpeg.c ... I really don't see the need
to add yet another source file just for this one function.

> It'd probably be feasible to do the expansion from (8-bit greyscale|24-bit RGB)
> to 32-bit RGBA just after we decompress a scanline, avoiding the fixup loop
> afterwards?

Yeah. You can do that probably.

> To be honest, this could benefit from a regression test. I'll try to find time
> to write one.

Please do!

> Increasing RGB_PIXELSIZE means we write 4 bytes for every 3 bytes read, which
> is a slightly cheating way to get a dummy alpha channel containing
> uninitialized data; tr_image_jpg.c goes through setting them all to 255 after
> the JPEG has been decompressed.

Well, let's get rid of that hack in the libjpeg included with ioquake3 so that
system jpeglib and ioq3 jpeglib is consistent in that. You can probably make
the expansion from 3 to 4 components at the same place where you expand from 1
to 4 components for 8-bit jpegs.

Remember: Don't forget to update the ioq3-changes.diff when you're done!

-- 
Configure bugmail: https://bugzilla.icculus.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.