Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add new OGR driver for OpenDRIVE (XODR) #9504

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

michikommader
Copy link
Contributor

@michikommader michikommader commented Mar 19, 2024

What does this PR do?

This adds a read-only vector driver for the road description format OpenDRIVE. OpenDRIVE is an open industry standard in the automotive domain of driving simulation, maintained by ASAM e.V. It is increasingly intersecting with the public and GIS domains which raises the need for better interoperability with open-source spatial tools.

  • This XODR driver is developed by German Aerospace Center (DLR) and licenced under MIT.
  • This XODR driver implementation uses libOpenDRIVE which is licenced under Apache 2.0.
  • This XODR driver is supposed to be used as plug-in.

I am am open to a fruitful discussion in order to make OpenDRIVE directly usable through GDAL. The context for this development has been introduced on FOSSGIS conference 2024: OpenDRIVE-HD-Karten mittels GDAL ins GIS bringen. Additional talks are planned for Geospatial World Forum and FOSS4G Europe 2024.

What are related issues/pull requests?

Tasklist

  • Make sure newly added files are correctly formatted (cf pre-commit configuration)
  • Add test case(s)
  • Add documentation
  • Updated Python API documentation (swig/include/python/docs/)
  • Clarify handling of custom docker/ubuntu-small/DockerfileXODR with GDAL maintainers: Which file location is better suited for future maintenance – the driver's source code directory or a separate repository?
  • Review
  • Adjust for comments
  • All CI builds and checks have passed

Environment

Provide environment details, if relevant:

  • OS: tested with Ubuntu 22.04, Windows 10
  • Compiler: GCC on Linux, GCC 13.1.0 x86_64 (MCF threads) MinGW-w64 MSVCRT on Windows

Copy link
Member

@rouault rouault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work generally. See my comments.

I would also like this to be triggered more by our CI.
Can you add a libopendrive build in .github/workflows/ubuntu_20.04/Dockerfile.ci and .github/workflows/ubuntu_22.04/Dockerfile.ci

Do you know how your driver (and libopendrive) is robust to corrupted / hostile datasets? At the very minimum, I'd like to see a test with a "random file" that has xodr extension, but isn't valid XODR file. Ideally we'd want to stress-test it using OSSFuzz. Cf https://github.com/OSGeo/gdal/blob/master/fuzzers/README.TXT , https://github.com/OSGeo/gdal/blob/master/fuzzers/build.sh

autotest/ogr/ogr_xodr.py Outdated Show resolved Hide resolved
else:
assert (
wkt == "POINT (618366.942790883 5809541.22374025 103.556888074495)"
), "wrong geometry created for dissolved RoadSignal"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be worth adding a test that runs the "test_ogrsf" compliance checking utility on the sample file. Something like:

def test_ogr_xodr_test_ogrsf():

    import test_cli_utilities

    if test_cli_utilities.get_test_ogrsf_path() is None:
        pytest.skip()

    ret = gdaltest.runexternal(
        test_cli_utilities.get_test_ogrsf_path() + " -ro " + xodr_file
    )

    assert "INFO" in ret
    assert "ERROR" not in ret


XODR -- OpenDRIVE Road Description Format
=========================================

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. versionadded:: 3.9

Specification version
---------------------

The currently supported OpenDRIVE version is 1.4 and basically depends on what is provided by libOpenDRIVE_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The currently supported OpenDRIVE version is 1.4 and basically depends on what is provided by libOpenDRIVE_.
The currently supported OpenDRIVE version is 1.4 and depends on what is provided by libOpenDRIVE_.

doc/source/drivers/vector/xodr.rst Show resolved Hide resolved
ogr/ogrsf_frmts/xodr/ogrxodrlayerroadmark.cpp Show resolved Hide resolved
ogr/ogrsf_frmts/xodr/ogrxodrlayerroadmark.cpp Show resolved Hide resolved
@rouault
Copy link
Member

rouault commented Mar 19, 2024

Is the some packaging of libopendrive in Debian, conda-forge, etc ?

@rouault
Copy link
Member

rouault commented Apr 18, 2024

@michikommader Do you plan any further work on this any time soon ? I'm asking as I'm trying to keep the list of opened PRs low, and if something is not going to progress for some time, it is better to close the PR, and re-open it later when work restart

@michikommader
Copy link
Contributor Author

@michikommader Do you plan any further work on this any time soon ?

Apologies for my inactivity. Yes, we already went through some of your valuable remarks internally but did not review and push changes yet. I plan to supply all the missing information in the first week of May.

@michikommader
Copy link
Contributor Author

Is the some packaging of libopendrive in Debian, conda-forge, etc ?

Not yet. I myself am not maintainer of libOpenDRIVE and would rather see packaging responsibility on the maintainer side. We can separately discuss if our institute could take over this task. Also, ASAM e. V. as standardisation organisation behind OpenDRIVE could have an interest in that.

@michikommader
Copy link
Contributor Author

Do you know how your driver (and libopendrive) is robust to corrupted / hostile datasets? At the very minimum, I'd like to see a test with a "random file" that has xodr extension, but isn't valid XODR file.

libOpenDRIVE forwards XML data loading directly to pugixml. According to pugixml's exception guarantees "most pugixml functions have a no-throw exception guarantee". For handling parsing errors the xml_parse_result would have to be inspected for its xml_parse_status. But, from GDAL driver side we do not have access to that. Unfortunately, libOpenDRIVE itself only uses simple printf() statements to report possible problems.

At least, libOpenDRIVE gives access to the parsed pugi::xml_document. Checking for the availability of "expected OpenDRIVE XML nodes" would allow basic validation if the parsing was successful. But, I don't see this as a very practicable way. If the dataset was corrupted "somewhere in the middle", pugixml would still have successfully parsed all previous content and made this accessible to the caller. For our use case of ensuring robustness in GDAL, I see this as a major deficit. The best solution would be to extend libOpenDRIVE to allow at least access to pugixml's xml_parse_status with which we could implement a somewhat meaningful error handling. I will approach the maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants