added: support building cximage with libpng 1.5. #601

Closed
wants to merge 1 commit into from

10 participants

@ghost

thanks to zenkibou / whoever is the original downstream (gentoo) dev.
closes ticket #12001

ideally this goes in eden to aid downstreams.

spiff added: support building cximage with libpng 1.5.
thanks to zenkibou / whoever is the original downstream (gentoo) dev.
closes ticket #12001
906108d
@wsoltys
Team Kodi member

Does it make sense to put some effort in using 1.5 on win32 as well?
Currently we're using the bundled 1.2.24 in our repo.

@theuni
Team Kodi member

Wow, yikes. -1 from me.

That's a big patch, and libpng is a pretty core dep. Additionally, I don't see 1.5 is any of the major distros. Looks like a patch gentoo should carry imo.

For master, we HAVE to ditch cximage once and for all for Frodo anyway.

@ghost

give me one argument not to support it. it's not like we're making it mandatory.

i'll be here all night.....................

@theuni
Team Kodi member

I have no problem with supporting 1.5, my concern is that it could affect <1.5. Many of the changes aren't straight ifdefs, but introduce new code for non-1.5 users (even if it should be functionally equal).

If you vouch for the code, I won't stand in your way. I just don't like seeing such a big change at this point (for Eden) that will only benefit a handful of users.

@fiveisalive

I'm the downstream maintainer of the Fedora/RPM Fusion package. I patched our version of Eden beta1 with the patch and compiled against external libpng-1.2.46 on f16, haven't seen any problems thus far. Our development version of Fedora rawhide/f17 has libpng 1.5 and works OK there according to reports (I don't have a rawhide box to test myself, however). Is there anyway I could test my Eden beta1 build with this patch when building against libpng 1.2.46?

@bobo1on1
Team Kodi member

Why all the #ifdef? Afaik all of those libpng functions have been available in libpng for years.

@Zenkibou

@theuni: I could do a new patch which would not change code for libpng<1.5.

@bobo1on1: The ifdef are here to avoid changing too much code for libpng<1.5 users.

I agree that cximage should be changed, what is the plan about that? Has this been discussed somewhere?
Using another generic image library (which one), or directly using the png/jpeg/bmp/whatever libraries?

@fiveisalive

It's a bit out of date now, but topfs2 created a patch to use ImageMagick in place of cximage, but there's not been much recent activity on that ticket:

http://trac.xbmc.org/ticket/8992

@lzoubek

I've compiled on gentoo against libpng-1.5.6 and I get segfaults everytime I visit image gallery and XBMC attempts to create thumbnails http://pastebin.com/RCn1KRNg

@Zenkibou

Good find !

I belive the pointer is not initialized to NULL, which can cause a segfault with non-paletted PNGs.
Can you try this fix:
Zenkibou@bc230b7

@Zenkibou

So this is fixed. Thanks lzoubek for testing !

@ssuominengentoo

Can we get this merged? Otherwise XMBC won't build for Gentoo, Fedora (rawhide), or Debian (experimental)

@Zenkibou

To make things more easy to merge (comment by theuni), I made a reworked patch which adds libpng 1.5 support without changing the existing libpng 1.3/1.4 code path.

Zenkibou@99d1031
(this includes the original libpng 1.5 patch, the first fix for uninitialized pointers and the second fix for png encoding reported by lzoubek)

@anssih
Team Kodi member

IMO we should apply the last patch from the previous comment, lots of distros are shipping libpng 1.5 (at least in their development versions, which will soon be released as stable).

The patch doesn't change any pre-1.5 codepaths so it should be safe for all already-working systems, and it will fix compilation failure on libpng 1.5+ systems.

@bobo1on1
Team Kodi member

If that's the case then we either do it now, or we end up doing it in 6 months time.

@gnif
Team Kodi member

+1 to adding this now, I was just hit with this exact problem also

@gnif
Team Kodi member

I can confirm that the patch by Zenkibou works perfectly, so bobo1on1, whenever your ready :)

@jmarshallnz
Team Kodi member

Agreed with Anssi + bobo1on1. The above patch does not alter pre v1.5 codepaths at all.

The only thing that would be nice is to get rid of the 2 or 3 #if PNG_LIBPNG_VER <= 10499 lines which appear unnecessary given that the code it refers could just be moved down into the #else blocks below.

@bobo1on1: want to handle pulling this in?

@bobo1on1
Team Kodi member

Sure.

@bobo1on1
Team Kodi member
@bobo1on1 bobo1on1 closed this Mar 17, 2012
@bobo1on1
Team Kodi member
@jmarshallnz
Team Kodi member

I meant this commit:

Zenkibou@99d1031

which didn't touch the existing codepaths?

@bobo1on1
Team Kodi member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment