mansoor-ahmed / openjpeg

Automatically exported from code.google.com/p/openjpeg
Other
0 stars 0 forks source link

[PATCH - v2 branch ] Internal function names conflict with Jasper #30

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I've developped a OpenJPEG based driver for GDAL. In the process of testing
it with other GDAL drivers enabled, I got crashes when both libopenjpeg and
libjasper are linked with GDAL. I've then realized that there's a name
conflict with symbols from Jasper... Namely jp2_decode and jp2_encode.
Those symbols are public symbols for Jasper, but internal ones for
OpenJPEG. So the attached patch adds a opj_ prefix in front of them, to
avoid segmentation faults when running GDAL autotests...

Original issue reported on code.google.com by even.rou...@gmail.com on 16 May 2010 at 9:35

Attachments:

GoogleCodeExporter commented 9 years ago
Please, fix it as soon as possible.
It took me 2 days to detect this problem in my program and I'm very angry!
Every Qt application that is linked with libopenjpeg and runs in the system, 
where KDE is installed, can potentionally crash, cause it loads kimg_jp2.so 
plugin, linked with libjasper.

Original comment by volkov...@gmail.com on 21 Dec 2010 at 2:30

GoogleCodeExporter commented 9 years ago
All *external* openjpeg V2 functions have an 'opj' prefix. IMHO, libjasper has 
not been cautious enough by defining external functions without a jasper 
prefix. 

If we apply the provided patch and want to remain consistent, we should 
actually rename all openjpeg functions with an 'opj' prefix. Do you guys see 
another less invasive solution ?

Original comment by antonin on 22 Dec 2010 at 10:41

GoogleCodeExporter commented 9 years ago
I disagree with the first two comments. C++ namespace were designed for that. 
For C project this is an issue application developer needs to work with.

In GDCM we use the following mangling:

http://gdcm.git.sourceforge.net/git/gitweb.cgi?p=gdcm/gdcm;a=blob;f=Utilities/gd
cmopenjpeg/openjpeg_mangle.h.in;h=62f15ece8340cb402c319d9e756c3f30be01f921;hb=HE
AD

Original comment by mathieu.malaterre on 22 Dec 2010 at 1:51

GoogleCodeExporter commented 9 years ago
Selon openjpeg@googlecode.com:

I didn't mention that I met the issue on Linux where the fact that symbols are
internal or external isn't really taken into account by the dynamic
loader/symbol resolver (internal symbols are still visible from the outside,
unless you use the visibility option of GCC4, but even in that case I've found
that it doesn't solve all situations). On Windows, this issue doesn't probably
exist however.

The symbol mangling done for GDCM isn't really an option for GDAL, where we
don't embed openjpeg code in our source tree, but rely on using the "official"
source code that the user has built independantly of GDAL.

I agree that the Jasper devs have probably not been very careful about the
namespace pollution, but as the jp2_decode and jp2_encode symbols are public for
Jasper and private for OpenJpeg, it seems more reasonable to rename them in
OpenJpeg... Now, do you need to rename all openjpeg functions with an 'opj'
prefix ? Well, it is up to you. It seems to be a pretty big change. Basically I
compared the output of nm on libjasper.so and libopenjpeg.so, and the only
intersection was jp2_encode and jp2_decode.

Original comment by even.rou...@gmail.com on 22 Dec 2010 at 2:10

GoogleCodeExporter commented 9 years ago
Jasper seems to be a dead project. Actually, that library sucks, it can easily 
crash you program.
I think, the future belongs to openjpeg, so you must add 'opj' prefix to all 
its functions.
I can make a complete patch. Just promise, that it will be applied :)

Original comment by volkov...@gmail.com on 22 Dec 2010 at 9:43

GoogleCodeExporter commented 9 years ago
nice, nobody cares (

Original comment by volkov...@gmail.com on 4 Jan 2011 at 5:02

GoogleCodeExporter commented 9 years ago
volkov...@gmail.com : can you still make a complete patch (for both trunk and 
v2) ?

Original comment by antonin on 29 Jan 2011 at 2:23

GoogleCodeExporter commented 9 years ago
I'll try this week

Original comment by volkov...@gmail.com on 30 Jan 2011 at 6:46

GoogleCodeExporter commented 9 years ago
here is the patch for trunk
please let me know if it is suitable

Original comment by volkov...@gmail.com on 1 Feb 2011 at 9:00

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks a lot. 

It seems to work ok for libopenjpeg but the patch didn't succeed in folders 
jp3d, JavaOpenJPEG and OPJViewer. Maybe a problem of file encoding ... I'll 
investigate this and keep you posted. Thanks again.

Original comment by antonin on 1 Feb 2011 at 9:30

GoogleCodeExporter commented 9 years ago
I cannot manage to patch the JP3D folder (see patchreport attached). 
Do you have an idea on how to solve this ?

Original comment by antonin on 1 Feb 2011 at 9:45

Attachments:

GoogleCodeExporter commented 9 years ago
I tried to check out from trunk and apply the patch.
It works well.
Maybe you need to do some synchronization?

Original comment by volkov...@gmail.com on 2 Feb 2011 at 10:58

GoogleCodeExporter commented 9 years ago
Oops, I see, something wrong with the encoding.
I'll try to fix it.

Original comment by volkov...@gmail.com on 7 Feb 2011 at 9:40

GoogleCodeExporter commented 9 years ago
thanks ! I wait for an updated patch then.

Original comment by antonin on 9 Feb 2011 at 10:44

GoogleCodeExporter commented 9 years ago
Fixed for now the conflicting function (jp2_encode and jp2_decode). 

volkov0aa, your patch is still welcome however :-).

Cheers,

Antonin

Original comment by antonin on 12 Apr 2011 at 5:26

GoogleCodeExporter commented 9 years ago

Original comment by mathieu.malaterre on 29 May 2012 at 3:43

GoogleCodeExporter commented 9 years ago
even, I am tempted to close this bug. This should be handled now, within issue 
#177

Original comment by mathieu.malaterre on 1 Oct 2012 at 3:35

GoogleCodeExporter commented 9 years ago
I think we can close it now with the 2.0 release
all the public api used opj_ prefix now and the great majority of the ABI also

Original comment by savmick...@gmail.com on 28 Nov 2012 at 12:54

GoogleCodeExporter commented 9 years ago

Original comment by antonin on 16 Sep 2014 at 11:41