On Tue, Apr 9, 2013 at 6:50 PM, Even Rouault <even.roua...@mines-paris.org>wrote:
> Le mardi 09 avril 2013 20:34:40, Even Rouault a écrit : > > Le mardi 09 avril 2013 19:06:28, Etienne Tourigny a écrit : > > > I have committed new warping methods average and mode to trunk, this > will > > > be part of gdal-1.10 > > > > Hi Etienne, > > > > It would be good if you could extend the autotest suite to add tests for > > those new warping methods. For that, you can likely take inspiration from > > the first tests of autotest/warp/warp.py. "Reference" images based on > > utmsmall.tif reference image is a bit big, but you can likely start from > a > > smaller source image like byte.tif instead that will produce reference > > images of reasonable size to be put in svn. > > > > Regarding nAlgo == 2 (mode with foating point data), the allocations of > > pafVals and panSums have the potential to fail if warping is done on a > > large image whose floating point values are rarely identical. So I think > > that VSIRealloc shoud be used instead with a test on the result to fail > > properly. I'm also a bit doubtfull about the practical usefulness of this > > case on real data. There might also be a performance issue due to the > loop > > "//Check array for existing entry" that is at the most inner level of the > > algorithm. > > I stand corrected on the above comment about big memory consumption. The > size > of the array is limited to the number of source pixels needed to compute a > target pixel, so unless you do extreme subsampling, that should be OK. > yes > > I've just noticed however that pafVals and panSums aren't free'd, so > there's a > memory leak currently. And the CPLRealloc() are a bit weird as currently > coded > : > > > int nMaxNumPx = 0; > float* pafVals = NULL; > int* panSums = NULL; > > if (nNumPx > nMaxNumPx) > { > pafVals = (float*) CPLRealloc(pafVals, nNumPx * > sizeof(float)); > panSums = (int*) CPLRealloc(panSums, nNumPx * > sizeof(int)); > nMaxNumPx = nNumPx; > } > > The test is always true, so CPLMalloc() would be clearer. But I think there > was a will to move nMaxNumPx, pafVals, panSums before the top loops. So > that > should likely be done. > I thought it is weird also, but again copied over from overview code. I thought about running valgrind, but then forgot. I will take your suggestions into consideration, thanks!
_______________________________________________ gdal-dev mailing list gdal-dev@lists.osgeo.org http://lists.osgeo.org/mailman/listinfo/gdal-dev