Skip to content
Snippets Groups Projects

Compatibility with gdal 3

Merged Compatibility with gdal 3
compatibility_gdal3 into develop
All threads resolved!
Merged Cédric Traizet requested to merge compatibility_gdal3 into develop
All threads resolved!

Summary

This merge request includes change to make OTB compatible with gdal 3.0

Rationale

Two main changes are introduced by this merge request:

  • A change on how ogr spatial references are compared during regression. Before this merge request the test spatial reference was compared with the baseline spatial reference by doing an ASCII comparison on a wkt representation of the SRS (using ogr's ExportToPrettyWkt method to generate the wkt). This is a very bad way to compare spatial references are different wkt string can represent the same SRS, and different version of gdal output different wkt representation (the "pretty wkt" of gdal 3.0 are a bit different of those of gdal 2, and in the future gdal will output wkt2 strings). Instead, with this MR, spatial references are compared with the ogr sptial reference IsSame method (i.e. we are delegating the comparison to gdal).

  • in gdal 3.0, we have to specify how spatial references should handle axis order. In OTB case we want to use the "traditional GIS" order (as it done extensively in ogr drivers), i.e. longitude/latitude for geographic CRS and east/north for projected CRS. More info here

  • Fix warning from importFromWkt() (const correctness)

Tests

There are three test failing with gdal 3.0:

  • prTvImageToEnvelopeVectorDataFilter
  • ioTvKMLVectorDataFileGeoReaderWriter
  • ioTvKMLVectorDataFileGeoReaderWriter2

These tests are using the kml format, the axis used by the ogr kml driver is not correct in gdal release 3.0. But this have been fixed since then, and the test are passing with the master of gdal (see this discussion for details)

Copyright

The copyright owner is CNES and has signed the ORFEO ToolBox Contributor License Agreement.

Check before merging:

  • All discussions are resolved
  • At least 2 :thumbsup: votes from core developers, no :thumbsdown: vote.
  • The feature branch is (reasonably) up-to-date with the base branch
  • Dashboard is green
  • Copyright owner has signed the ORFEO ToolBox Contributor License Agreement
  • Optionally, run git diff develop... -U0 --no-color | clang-format-diff.py -p1 -i on latest changes and commit
Edited by Cédric Traizet

Merge request reports

Merge request pipeline #1698 passed

Stage: precheck
Stage: prepare
Stage: build

Merge request pipeline passed for a9ad9c59

Merged by Cédric TraizetCédric Traizet 5 years ago (Jun 13, 2019 12:14pm UTC)

Loading

Pipeline #1744 passed

Stage: precheck
Stage: external

Pipeline passed for 7b09c7e4 on develop

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Cédric Traizet changed milestone to %7.0.0

    changed milestone to %7.0.0

  • Cédric Traizet added CNES backlog + 1 deleted label

    added CNES backlog + 1 deleted label

  • Cédric Traizet changed the description

    changed the description

  • Cédric Traizet
    Cédric Traizet @ctraizet
    Author Developer

    I don't know how this should be tested, as out platforms (Dashboard and CI) are using gdal 2.0. Here the CI uses gdal 2, so it only backward compatibility.

    If we want to test it with gdal 3.0 on all platforms, we should update the SuperBuild gdal version of this branch to 3.0 (or master), but this will require some additional work on GDAL patches...

  • Victor Poughon
    Victor Poughon @poughov
    Contributor

    IMO it's enough if all CI test GDAL 2.x and you have ran all tests with a local build of GDAL 3.0. In the future we should gradually upgrade CI platforms to GDAL 3.x and if any problem comes up then it will be treated as a normal bug.

  • Cédric Traizet changed the description

    changed the description

  • Cédric Traizet
    Cédric Traizet @ctraizet
    Author Developer

    Using my local GDAL 3.0 and PROJ4 6 build, all test are successful if the otb is compiled in release mode, but if I compile otb in debug mode, many tests are failing because of a segfault. I do not understand why it happens yet. It seems related to the GDALImageIO when using the geotiff driver and proj 6. This is what valgrind throws:

    valgrind /Datas/Code/OTB/build3/bin/otbTestDriver "--compare-image" "0.0" "/Datas/Code/OTB/otb/Data/Baseline/OTB/Images/apTvSeSegmentationWatershedRaster.tif" "/Datas/Code/OTB/build3/Testing/Temporary/apTvSeSegmentationWatershedRaster.tif" "Execute" "/Datas/Code/OTB/build3/bin/otbApplicationLauncherCommandLine" "Segmentation" "/Datas/Code/OTB/build3/lib/otb/applications" "-in" "/Datas/Code/OTB/otb/Data/Input/WV2_MUL_ROI_1000_100.tif" "-filter" "watershed" "-mode" "raster" "-mode.raster.out" "/Datas/Code/OTB/build3/Testing/Temporary/apTvSeSegmentationWatershedRaster.tif" "uint16" "-testenv"
==15365== Memcheck, a memory error detector
==15365== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==15365== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==15365== Command: /Datas/Code/OTB/build3/bin/otbTestDriver --compare-image 0.0 /Datas/Code/OTB/otb/Data/Baseline/OTB/Images/apTvSeSegmentationWatershedRaster.tif /Datas/Code/OTB/build3/Testing/Temporary/apTvSeSegmentationWatershedRaster.tif Execute /Datas/Code/OTB/build3/bin/otbApplicationLauncherCommandLine Segmentation /Datas/Code/OTB/build3/lib/otb/applications -in /Datas/Code/OTB/otb/Data/Input/WV2_MUL_ROI_1000_100.tif -filter watershed -mode raster -mode.raster.out /Datas/Code/OTB/build3/Testing/Temporary/apTvSeSegmentationWatershedRaster.tif uint16 -testenv
==15365== 
2019-06-11 16:08:58 (INFO): Default RAM limit for OTB is 256 MB
2019-06-11 16:08:58 (INFO): GDAL maximum cache size is 794 MB
2019-06-11 16:08:58 (INFO): OTB will use at most 1 threads
2019-06-11 16:08:58 (INFO) Segmentation: Default RAM limit for OTB is 256 MB
2019-06-11 16:08:58 (INFO) Segmentation: GDAL maximum cache size is 794 MB
2019-06-11 16:08:58 (INFO) Segmentation: OTB will use at most 1 threads
2019-06-11 16:08:58 (INFO): Loading kwl metadata from attached geom file /Datas/Code/OTB/otb/Data/Input/WV2_MUL_ROI_1000_100.geom
2019-06-11 16:08:58 (INFO) Segmentation: Using watershed segmentation.
2019-06-11 16:08:58 (INFO) Segmentation: Segmentation mode which output label image.
2019-06-11 16:08:58 (INFO): Estimated memory for full processing: 0.133514MB (avail.: 256 MB), optimal image partitioning: 1 blocks
2019-06-11 16:08:58 (INFO): File /Datas/Code/OTB/build3/Testing/Temporary/apTvSeSegmentationWatershedRaster.tif will be written in 1 blocks of 100x100 pixels
Writing /Datas/Code/OTB/build3/Testing/Temporary/apTvSeSegmentationWatershedRaster.tif...: 100% [**************************************************] (0s)
 -> Test EXIT SUCCESS.
-------------  Start control baseline tests    -------------
Number of baseline images: 1
Testing non-regression on image: /Datas/Code/OTB/otb/Data/Baseline/OTB/Images/apTvSeSegmentationWatershedRaster.tif
==15365== Invalid read of size 8
==15365==    at 0x1272E131: getDBcontext(projCtx_t*) (c_api.cpp:117)
==15365==    by 0x1272E1B1: getDBcontextNoException(projCtx_t*, char const*) (c_api.cpp:128)
==15365==    by 0x1272E2DF: pj_obj_create(projCtx_t*, dropbox::oxygen::nn<std::shared_ptr<osgeo::proj::common::IdentifiedObject> > const&) (c_api.cpp:139)
==15365==    by 0x1273989C: proj_create_conversion (c_api.cpp:3015)
==15365==    by 0x7555B47: OGRSpatialReference::SetProjCS(char const*) (ogrspatialreference.cpp:4537)
==15365==    by 0x7792667: GTIFGetOGISDefn (gt_wkt_srs.cpp:506)
==15365==    by 0x774C8BA: GTiffDataset::LookForProjection() (geotiff.cpp:12672)
==15365==    by 0x7750901: GTiffDataset::GetSpatialRef() const (geotiff.cpp:18092)
==15365==    by 0x7A45859: GDALDataset::GetProjectionRef() const (gdaldataset.cpp:851)
==15365==    by 0xAFB282A: otb::GDALImageIO::InternalReadImageInformation() (otbGDALImageIO.cxx:763)
==15365==    by 0xAFB057F: otb::GDALImageIO::ReadImageInformation() (otbGDALImageIO.cxx:332)
==15365==    by 0x6A283CF: otb::ImageFileReader<otb::VectorImage<double, 2u>, otb::DefaultConvertPixelTraits<double> >::GenerateOutputInformation() (otbImageFileReader.hxx:309)
==15365==  Address 0x1d0366b0 is 0 bytes after a block of size 32 alloc'd
==15365==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15365==    by 0x9B3310A: pj_ctx_alloc (in /usr/lib/x86_64-linux-gnu/libproj.so.9.1.0)
==15365==    by 0x125366CD: proj_context_create (4D_api.cpp:1306)
==15365==    by 0x74FC188: OSRPJContextHolder::init() (ogr_proj_p.cpp:84)
==15365==    by 0x74FC1F5: __tls_init (ogr_proj_p.cpp:102)
==15365==    by 0x74FC248: UnknownInlinedFun (ogr_proj_p.cpp:102)
==15365==    by 0x74FC248: OSRGetProjTLSContext() (ogr_proj_p.cpp:106)
==15365==    by 0x773EB9B: GTiffDatasetGTIFNew(tiff*) (geotiff.cpp:10973)
==15365==    by 0x774D65A: GTiffDataset::LoadGeoreferencingAndPamIfNeeded() (geotiff.cpp:14550)
==15365==    by 0x7747687: GTiffDataset::GetMetadataItem(char const*, char const*) (geotiff.cpp:18480)
==15365==    by 0x7A509B9: GDALDefaultOverviews::OverviewScan() (gdaldefaultoverviews.cpp:316)
==15365==    by 0x7A512B8: GDALDefaultOverviews::IsInitialized() (gdaldefaultoverviews.cpp:120)
==15365==    by 0x7A80945: GDALRasterBand::GetOverviewCount() (gdalrasterband.cpp:2186)
==15365==
  • Cédric Traizet
    Cédric Traizet @ctraizet
    Author Developer

    I tried again with clean proj and gdal builds (both using the master branch) and I don't have segfaults anymore. I think it was caused by gdal third party libraries built with a proj library different from 6 (e.g. all third parties coming from the package manager). This time I followed the gdal specific guidelines for version 3.0 build.

    So I think this can be merged.

    (all test are passing on release, and the classification tests are failing on debug because of this issue with the Field::GetValue<T> method)

  • Victor Poughon
    Victor Poughon @poughov
    Contributor

    ok for me. if we can get one more review it would be nice though. (@aregimbeau, @gpasero or @jmichel?)

  • Julien Radoux
    Julien Radoux @radouxju

    not yet familiar with the testing procedure, but this is a great improvement as I add previous issue with this SRS compatibility (even within the same app, e.g. otbcli_KMeansClassification with input raster in EPSG:31370). My only workaround was to define a "false" UTM projection, run the app the change to the correct SRS after the classification.

  • Cédric Traizet resolved all discussions

    resolved all discussions

  • Cédric Traizet merged

    merged

  • Cédric Traizet mentioned in commit 7b09c7e4

    mentioned in commit 7b09c7e4

  • Cédric Traizet added refactoring label

    added refactoring label

  • Cédric Traizet mentioned in issue #1980 (closed)

    mentioned in issue #1980 (closed)

Please register or sign in to reply