Opened 12 years ago

Closed 10 years ago

Last modified 10 years ago

#4331 closed enhancement (fixed)

Adding SXF file format driver

Reported by: bidandou Owned by: bidandou@…
Priority: normal Milestone:
Component: OGR_SF Version: unspecified
Severity: normal Keywords: SXF
Cc: warmerdam, dron, rutsky

Description (last modified by dron)

the goal of the work is to add a GDAL driver supporting the SXF (Storage and eXchange Format). This file format is largely used in the eastern europe, and is primarly produced by a GIS software Called Panorama GIS MAP developped by the Russian KB Panorama Entreprise
informationons on : http://www.gisinfo.net/
Specification on (in Russian Language) : http://gistoolkit.ru/download/doc/sxf4bin.pdf

Attachments (13)

drv_sxf.html (302 bytes ) - added by bidandou 12 years ago.
makefile.vc (194 bytes ) - added by bidandou 12 years ago.
GNUmakefile (224 bytes ) - added by bidandou 12 years ago.
ogr_sxf.h (7.1 KB ) - added by bidandou 12 years ago.
ogrsxfdatasource.cpp (12.2 KB ) - added by bidandou 12 years ago.
ogrsxfdriver.cpp (3.2 KB ) - added by bidandou 12 years ago.
org_sxf_defs.h (14.8 KB ) - added by bidandou 12 years ago.
sxf.h (60 bytes ) - added by bidandou 12 years ago.
sxf.2.h (60 bytes ) - added by bidandou 12 years ago.
systypes.h (60 bytes ) - added by bidandou 12 years ago.
ogrsxflayer.cpp (20.2 KB ) - added by bidandou 12 years ago.
sxf.patch (103.9 KB ) - added by bishop 10 years ago.
Patch to current trunk add sxf driver
sxf_hardening.patch (21.8 KB ) - added by Even Rouault 10 years ago.

Download all attachments as: .zip

Change History (48)

by bidandou, 12 years ago

Attachment: drv_sxf.html added

by bidandou, 12 years ago

Attachment: makefile.vc added

by bidandou, 12 years ago

Attachment: GNUmakefile added

comment:1 by Even Rouault, 12 years ago

Cc: warmerdam dron added

I have several high level remarks and questions :

  • For significant contributions like a new driver, we'd ask that you publicly declare in an email to gdal-dev mailing list that your contribution matches the requirements of the "Legal" paragraph of http://trac.osgeo.org/gdal/wiki/rfc3_commiters
  • Do you have rights to publish sxf.h under the X/MIT licence ? There's no copyright notice in the file, and I've got under the impression that it might come from another project. Please correct me if I'm wrong.
  • Is there a public specification of the format somewhere ?
  • You should put your full name in the copyright notice of the files, not the osgeo id.
  • The first thing to fix is the indendation of the files. Please don't use tab characters, but use instead 4 spaces for each indentation level. See http://trac.osgeo.org/gdal/wiki/rfc8_devguide
  • Remove dead code that is commented
  • Remove printf statements. You can use CPLDebug infrastructure if needed
  • Do you have links to SXF datasets that could be used to test your contribution ? Ideally we would need a freely redistribuable and small datasource so that a regression test can be written.
  • With which OS did you test your driver : Linux, Windows ?
  • Your code will only work on little-endian processors. Not a critical issue for now however...

Note : to avoid the proliferation of files when you update new revisions of your work, you can overwrite existing files attached to the ticket by checking the "Replace existing attachment of the same name" box.

CC'ing Andrey Kiselev who has contributed stuff in GDAL for Panorama SRS

Merci de votre contribution !

comment:2 by bidandou, 12 years ago

Description: modified (diff)

comment:3 by bidandou, 12 years ago

Description: modified (diff)

comment:4 by bidandou, 12 years ago

Description: modified (diff)

comment:5 by bidandou, 12 years ago

Description: modified (diff)

comment:6 by bidandou, 12 years ago

SXF datasets that could be used to test the contribution : some SXF files exist on thos URL :
http://gistoolkit.ru/download/

the most interesting thing is the availability of a free downladable softwares in the above site alowing the cration and the export of SXF files in different configurations to do tests.

see: http://www.gisinfo.net/download/download.htm

for the OS I used to test my driver : Windows XP SP3 and Windows 7?

I'm Using Visual Studio 2008 Expess Edition for the developpement. dont try to compile with gcc.

I submit the files above just to know how does mechanism of contriburions work. My work is not finished.

Thank you for your interest, soon I'll correct my work and I'll submit more comprehensible code.

by bidandou, 12 years ago

Attachment: ogr_sxf.h added

by bidandou, 12 years ago

Attachment: ogrsxfdatasource.cpp added

by bidandou, 12 years ago

Attachment: ogrsxfdriver.cpp added

by bidandou, 12 years ago

Attachment: org_sxf_defs.h added

by bidandou, 12 years ago

Attachment: sxf.h added

by bidandou, 12 years ago

Attachment: sxf.2.h added

by bidandou, 12 years ago

Attachment: systypes.h added

by bidandou, 12 years ago

Attachment: ogrsxflayer.cpp added

comment:7 by Even Rouault, 12 years ago

Ali, (sorry if I misidentified your first name ?)

Thanks for the updates.

comment:8 by bidandou, 12 years ago

Yes you are right, the ogr_sxf_defs.h comes from the spec of the SXF file format transtated to english.

you can find an SXF file in :

http://gistoolkit.ru/download/examplemap.zip

For now I consider that my work is not ready enough, there still some importants thinks to do, so I'd like to have time to do that.

Thank you for you assistance

comment:9 by dron, 12 years ago

Description: modified (diff)

comment:10 by dron, 12 years ago

Description: modified (diff)

comment:11 by dron, 12 years ago

Hi,

Just found a time to look into this issue.

I should say that I have a code for SXF format developed by me several years ago, but it is still out of OGR tree because there were no pressing need for that driver (and the OGC SF model is not sufficient for SXF, so I developed a separate library for SXF reading and writing to be able to explore all the features of the format). Moreover, I actually have two drivers: the one based on SDK from KB Panorama, and another one with my own code only. I should note that official format spec is bad and vague and a bit of experimentation and reverse engineering is needed to properly read SXF files.

There is a separate contribution now, so I am bit late with my own stuff, but it is certainly a time to introduce this new driver. So we need to come to some solution how we will proceed. I can invest some time for this development and push my code too. Though I don't want to put down a new contributor I should note that my code supports more SXF variations and goes further. Also it is architecture and compiler independent.

Best regards, Andrey

in reply to:  11 comment:12 by bidandou, 12 years ago

Hi,

I'm happy to see that idea of the introduction of the new SXF driver is well accepted by you. If you got code that can drive the development of the driver forward you're invited to contribute with me if you want, even if you want to take the lead of the development process, I have no objection.

The most important to me is the introduction of this new driver in GDAL, and making it accessible to all the people that want it.

Elsewhere, if you haven't enough time, you can send me the code you developed and I will introduce it my own work (with your copyright of course).

Best regards, Ali

comment:13 by dron, 12 years ago

I am working on integration of my code in GDAL now, I hope it will be ready at the end ow the week. I will put the code here for review after that.

Best regards, Andrey

comment:14 by warmerdam, 12 years ago

Andrey, will this be in for Beta2 in the next few days? Or should we defer this to post 1.9?

in reply to:  14 comment:15 by dron, 12 years ago

Replying to warmerdam:

Andrey, will this be in for Beta2 in the next few days? Or should we defer this to post 1.9?

Definitely it should go in the next release. It will need testing and maybe some improvements.

comment:16 by bidandou, 12 years ago

Milestone: 1.9.02.0.0

comment:17 by bidandou, 12 years ago

Hi Andrey,

I see that nothing is changing about SXF driver and no new code is posted to the gdal tree.

If you are not interested on working on that driver, please tell me so I continue my work.

Best regards, Ali.

comment:18 by bidandou, 12 years ago

Hi warmerdam,

I see that nothing is changing about SXF driver and no new code is posted to the gdal tree from more than 10 months.

I want to know if I can work again on that driver, please tell me if I can continue my work.

Best regards, Ali.

by bishop, 10 years ago

Attachment: sxf.patch added

Patch to current trunk add sxf driver

comment:19 by bishop, 10 years ago

This is the major rewrite of the sxf driver, tested with various SXF samples. This work was done by us at NextGIS for Vega corp. I think the driver is ready to be included in the next release. We are offering to maintain it in the future. Let me know what next steps should be.

comment:20 by Even Rouault, 10 years ago

Dmitry, you're a GDAL committer, so you can just push the code.

Just skimming quickly through the code, I see that you will need to add the registration of the driver in OGRAllRegister() and add the driver in the Unix/Windows makefiles. See r26470 (for example) for the list of files that need to be modified.

If you have data that is small enough to be put in SVN (ideally < 100K) and with an appropriate license, it would be good to add a few tests in autotest/ogr.

I don't know if you've already used it, but you can also use the "test_ogrsf" utility to test the driver behaviour w.r.t. OGR API expectations. You must build it explicity with "make test_ogrsf" in the app directory.

I see a few occurences of CPLError( CE_Fatal . This will abort the process (which can be annoying in a desktop application). It is generally better to issue a CE_Failure and error out more cleanly.

I also see use of dereferencing of items in buffer with patterns like "*dfX = *(double *)(psBuff + 8)". This would cause portability issue on non-Intel platforms that are big-endian and/or have strict pointer alignment requirements. memcpy() + CPL_LSBxxxx macros in cpl_port.h can be used for that purpose.

comment:21 by bishop, 10 years ago

Thanks Even for you review. I'll going to fix all this and commit.

comment:22 by rutsky, 10 years ago

Cc: rutsky added

comment:23 by bishop, 10 years ago

Resolution: fixed
Status: newclosed

Add SXF driver in r26764

comment:24 by Even Rouault, 10 years ago

r26765 "SXF: fix cross compilation issue with i586-mingw32msvc (#4331)"

r26766 "SXF: fix memory leaks (#4331)"

comment:25 by Even Rouault, 10 years ago

r26767 "SXF: fix potential problem on Linux 64bit where long is 64 bit (#4331)"

comment:26 by Even Rouault, 10 years ago

r26768 "SXF: fix so that GetNextFeature() works directly (fix ogrinfo -q) (#4331)"

comment:27 by Even Rouault, 10 years ago

r26770 "SXF: add explicit padding byte in SXFRecordHeader structure, so that it is 32 bytes with MSVC (#4331)"

comment:28 by bishop, 10 years ago

r26771 Add several new projections support to SXF driver (#4331)

comment:29 by Even Rouault, 10 years ago

r26772 SXF: fix various random crashes on Windows that were due to the /Zp1 option, forcing packing of structures to 1 byte alignment, but causing issues with OGR core structures. Now the unit tests pass. If structure packing is still necessary, don't do it that way but with local #pragma (#4331)

comment:30 by Even Rouault, 10 years ago

r26773 SXF: fix crash when reading of SXF header errored. Check the .sxf extension before stat'ing the file (#4331)

comment:31 by Even Rouault, 10 years ago

Dmitry, I'm attaching a patch (sxf_hardening.patch) that fixes robustness issues against invalid data that have cause crashes while running the test data file in a fuzzing tool that generates random variations of the initial file. I've verified that the patch doesn't change the output of ogrinfo -al on the test data file, but you might want to check that the added checks don't cause functional regressions on other files.

by Even Rouault, 10 years ago

Attachment: sxf_hardening.patch added

comment:32 by bishop, 10 years ago

I applied patch. Everything is seems to be OK.

comment:33 by bishop, 10 years ago

r26786. Fix layers name encoding.

comment:34 by Even Rouault, 10 years ago

About r26786 there's a leak of pszRecoded around line 1202

The naming for szFontEnc is a bit inconsistant with the usual Hungarian conventions. Should rather be a nFontEnc I believe.

in reply to:  34 comment:35 by bishop, 10 years ago

Replying to rouault:

About r26786 there's a leak of pszRecoded around line 1202

The naming for szFontEnc is a bit inconsistant with the usual Hungarian conventions. Should rather be a nFontEnc I believe.

Fixed in r26788

Note: See TracTickets for help on using tickets.