Re: [PanoTools-devel] readImage

> why don't we do this: unify readImage into file.c? Delete it from
> bmp.c, pict.c and ppm.c. If something is not supported in one system
> ifdef it right there. We should also flag any other function that is
> duplicated in name. Duplication of functions tends to be create these
> types of problems.
>
I think this is a good approach...get rid of as much of the duplication as
possible and unify this in one place. I don't see why it wouldn't be possible
to support PPM and BMP images on all platforms.
As for the HDR images, I really have no idea what it is, who added it, how it
is supposed to work, and so on.
> I can do these changes if you want.
Yes, please! I made some changes to bmp.c (not checked in), but will get rid
of these in favor of merging the changes into file.c.
Thanks,
Max

Thread view

Hi Max,
I separated my reply into 2 messages because they deal with different issues.
I finally took a look at what the problem was. We have _3_ copies of
the function readImage! The bmp.c file does not even get compiled in
my system. Which means that HDR support is not present under OS
X. Perhaps what we need is to move readImage and writeImage out of all
these files and into just one single place.
Max> In any case, I will try and fix up the Windows code as much as possible. After
Max> reworking the readImage function, TIFF and JPEG input seem to work OK.
Max> However, BMP, PNG and HDR input are still completely broken. I can probably
Max> reuse the code in jpeg.c (the stuff commented with "should be considered a
Max> hack") for BMP and PNG input. I have no idea about the HDR stuff, but maybe
Max> someone else knows more about this.
why don't we do this: unify readImage into file.c? Delete it from
bmp.c, pict.c and ppm.c. If something is not supported in one system
ifdef it right there. We should also flag any other function that is
duplicated in name. Duplication of functions tends to be create these
types of problems.
We need to set the metadata after we read the input file, regardless
of format (for the time being, given that we do not support any other
type of data), but that will be the same function for all of them. I
can do these changes if you want.
Max> I also noticed that if I run a script for which the images don't exist I get a
Max> message saying "We only support 3 or 4 samples per pixel". This is
Max> misleading...we might want a message saying something like "Cannot open input
Max> image". The program also crashes at this point (I think it is trying to do a
Max> TIFFClose with a null reference).
I think it is also worth adding test cases that fail. We currently do
not regression test for that.
Max> Lastly...are there any other Windows developers who can compile this stuff "out
Max> of the box"? Or does everyone else have to fool around with editing/creating
Max> their own makefiles? I've never been able to get the autoconf/configure stuff
Max> to work. I've always had to roll my own makefiles. I now get an endless
Max> stream of multiple definition errors when trying to compile PTMender. It seems
Max> to be related to the tiff functions that are defined in pano12.dll, the tiff
Max> functions that are in libtiff and trying to compile ptmender with tiff.c and
Max> link to libtiff and pano12.dll at the same time. My plan is to build ptmender
As I inspect more and more of the code I realize there is a lot of
duplication (Helmut tended to include copies of other libraries inside
pano12). that is part of the reason I started renaming functions to
make it explicit they are part or not of libpano. As we finish this
round of changes we can remove a lot of code from the top of tiff.c.
--
Daniel M. German "Let us show our friendship for a man
when he is alive and not after
he is dead.
F. Scott Fitzgerald -> "
http://turingmachine.org/http://silvernegative.com/
dmg (at) uvic (dot) ca
replace (at) with @ and (dot) with .

> why don't we do this: unify readImage into file.c? Delete it from
> bmp.c, pict.c and ppm.c. If something is not supported in one system
> ifdef it right there. We should also flag any other function that is
> duplicated in name. Duplication of functions tends to be create these
> types of problems.
>
I think this is a good approach...get rid of as much of the duplication as
possible and unify this in one place. I don't see why it wouldn't be possible
to support PPM and BMP images on all platforms.
As for the HDR images, I really have no idea what it is, who added it, how it
is supposed to work, and so on.
> I can do these changes if you want.
Yes, please! I made some changes to bmp.c (not checked in), but will get rid
of these in favor of merging the changes into file.c.
Thanks,
Max

Hi Max,
Max Lyons twisted the bytes to say:
>> why don't we do this: unify readImage into file.c? Delete it from
>> bmp.c, pict.c and ppm.c. If something is not supported in one system
>> ifdef it right there. We should also flag any other function that is
>> duplicated in name. Duplication of functions tends to be create these
>> types of problems.
>>
Max> I think this is a good approach...get rid of as much of the duplication as
Max> possible and unify this in one place. I don't see why it wouldn't be possible
Max> to support PPM and BMP images on all platforms.
Max> As for the HDR images, I really have no idea what it is, who added it, how it
Max> is supposed to work, and so on.
>> I can do these changes if you want.
Max> Yes, please! I made some changes to bmp.c (not checked in), but will get rid
Max> of these in favor of merging the changes into file.c.
Done. All formats are now supported, but I can't compile BPM. It
requires windows headers. But I test it and it passes tests on my
side. Give it a try.
I have added 3 files: metadata.c, metadata.h and file.h so you will
have to update you makefile.
--
daniel
Max> Max
Max> -------------------------------------------------------------------------
Max> Take Surveys. Earn Cash. Influence the Future of IT
Max> Join SourceForge.net's Techsay panel and you'll get the chance to share your
Max> opinions on IT & business topics through brief surveys -- and earn cash
Max> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
Max> _______________________________________________
Max> PanoTools-devel mailing list
Max> PanoTools-devel@...
Max> https://lists.sourceforge.net/lists/listinfo/panotools-devel
--
Daniel M. German
http://turingmachine.org/http://silvernegative.com/
dmg (at) uvic (dot) ca
replace (at) with @ and (dot) with .