[mkgmap-dev] [Patch] Error in BoundaryUtil

Mon Mar 12 09:15:39 GMT 2012

Hi WanMil,

I don't see a need for the length field in the header, and I don't see how I could use it
(I found no way to calculate how many bytes were removed from a stream)

So I've coded this:
Format type: String: BND
Timestamp: long
 | Record format: String: (RAW, QUADTREE)
 | Record layout: int
 | mkgmap release: String
 ... Records ...

Attached is the patch that implements this.

Gerd

> Date: Sun, 11 Mar 2012 21:02:56 +0100
> From: wmgcnfg at web.de
> To: mkgmap-dev at lists.mkgmap.org.uk
> Subject: Re: [mkgmap-dev] [Patch] Error in BoundaryUtil
> 
> > Hi WanMil,
> >
> >
> >
> >  > Date: Sun, 11 Mar 2012 20:08:54 +0100
> >  > From: wmgcnfg at web.de
> >  > To: mkgmap-dev at lists.mkgmap.org.uk
> >  > Subject: Re: [mkgmap-dev] [Patch] Error in BoundaryUtil
> >  >
> >  > i Gerd,
> >  >
> >  > can you please explain in short what's the difference between the three
> >  > formats (my thinking):
> >  > * Legacy (original - int coords)
> >  > * Raw (float coords)
> >  > * Quadtree (admin level areas merged to the quadtree format)
> >
> > yes, correct
> >
> >  >
> >  > loadQuadTree (Quadtree format?) => loadQuadTreeFromStream (Quadtree
> >  > format?) => readStreamRawFormat (Raw format?) => readArea (Legacy format)
> >  >
> >  > At least it is irritating to me if there is a method readStreamRawFormat
> >  > which reads raw and/or legacy format.
> >
> > I agree. I'll change that so that we have two methods
> > readStreamLegacyFormat and readStreamRawFormat.
> 
> Great! Maybe it makes also sense to put the detection of the format into 
> one method? In the moment there are a lot of rel.endsWith(...) checks in 
> the code.
> 
> >
> >  >
> >  > I would also propose a change to the header of the new raw and quadtree
> >  > format. In the legacy format the first String in the file gives the
> >  > release of mkgmap. You added a "_raw" or "_quadtree" (?) to mark the new
> >  > formats. This is unclean but required to support the legacy format. But
> >  > legacy format support can be dropped in the future (I think at least in
> >  > the end of 2012). So to be open for future other changes it would be
> >  > good to have a kind of format identification. This could be added after
> >  > the creation time. Having this format identification future changes to
> >  > the format could be easily and much cleaner added.
> >
> > Yes, my problem with the legacy format was that it did not contain such
> > a format identifier.
> > I've worked a long time with programs on IBMs mainframe system (OS/390,
> > z/OS).
> > They typically use an eye catcher to identify the record type and a
> > numeric field that
> > identifies the record layout version. In some programs, they have also a
> > length field.
> > Old programs can read newer structures because the newer fields are only
> > added
> > at the end of a structure, and the length field allows to skip them.
> > I think this is a good idea, but we probably don't have to support all
> > kinds of
> > versions in programs and files.
> >
> > So, here is my suggestion:
> > instead of adding _raw or _quadtree to the 1st string I replace it with BND.
> > Legacy format is identified because it doesn't contain this string.
> > A new String field fileFormat after the timestamp identifies the file
> > format,
> > either RAW or QUADTREE.
> > One more int field recId identifies the record layouts used (0 for this
> > first version).
> > Whenever a new format is needed, we change fileFormat, if only a new
> > field is needed in e.g. the AREA section, we increase recId.
> > All methods that read the file verify that they are able to process the
> > recId
> > or they stop with an FormatException .
> > OK?
> 
> Sounds good. But I propose two additional things:
> 1. a header size or id (otherwise we will have the same problem again to 
> add an additional information to the header)
> 2. Keep an mkgmap release identifier.
> 
> So the new format would look like:
> Format type: String: BND
> Header size: int
> | Record format: String: (RAW, QUADTREE)
> | Record layout: int
> | Timestamp: long
> | mkgmap release: String
> ... Records ...
> 
> WanMil
> 
> 
> >
> > Gerd
> >
> >  >
> >  > WanMil
> >  >
> >  > > Hi WanMil,
> >  > >
> >  > > hmm, we have two errors here. I can confirm that my patch doesn't solve
> >  > > all problems,
> >  > > it just allows again to read the old *.bnd format and the new quadtree
> >  > > format, but mkgmap
> >  > > fails to read the intermediate format produced by the preparers 1st
> > pass.
> >  > >
> >  > > > Date: Sun, 11 Mar 2012 18:25:21 +0100
> >  > > > From: wmgcnfg at web.de
> >  > > > To: mkgmap-dev at lists.mkgmap.org.uk
> >  > > > Subject: Re: [mkgmap-dev] [Patch] Error in BoundaryUtil
> >  > > >
> >  > > > Hi Gerd,
> >  > > >
> >  > > > without your patch preparing boundaries works.
> >  > >
> >  > > No, it doesn't. It stops (crashes) without any error message when
> > trying
> >  > > to read the *.bnd
> >  > > files produced in the 1st pass. You can verify this by looking at the
> >  > > header of
> >  > > the new *.bnd files, they have the suffix _raw, not _quadtree.
> >  > >
> >  > > The new patch fixes this, but not the missing error detection.
> >  > >
> >  > > I think we need a try/catch block somewhere, either in Preparer or
> >  > > BoundaryPreparer.
> >  > > Will you take care of this?
> >  > >
> >  > > Gerd
> >  > >
> >  > >
> >  > > > Using your patch I get the following exception.
> >  > > >
> >  > > > java.util.concurrent.ExecutionException:
> >  > > > java.lang.IllegalArgumentException: Ill
> >  > > > egal Capacity: -572471432
> >  > > > at java.util.concurrent.FutureTask$Sync.innerGet(Unknown Source)
> >  > > > at java.util.concurrent.FutureTask.get(Unknown Source)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.main.Preparer.runPreparer(Preparer.java:92)
> >  > > > at uk.me.parabola.mkgmap.main.Main.endOptions(Main.java:349)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.CommandArgsReader.readArgs(CommandArgsReader.ja
> >  > > > va:126)
> >  > > > at uk.me.parabola.mkgmap.main.Main.main(Main.java:114)
> >  > > > Caused by: java.lang.IllegalArgumentException: Illegal Capacity:
> >  > > -572471432
> >  > > > at java.util.ArrayList.<init>(Unknown Source)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.readStreamRawF
> >  > > > ormat(BoundaryUtil.java:282)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.loadQuadTreeFr
> >  > > > omStream(BoundaryUtil.java:459)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.loadQuadTree(B
> >  > > > oundaryUtil.java:145)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.loadQuadTree(B
> >  > > > oundaryUtil.java:120)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryPreparer$QuadTreeWo
> >  > > > rker.call(BoundaryPreparer.java:236)
> >  > > > at
> >  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryPreparer$QuadTreeWo
> >  > > > rker.call(BoundaryPreparer.java:221)
> >  > > > at java.util.concurrent.FutureTask$Sync.innerRun(Unknown Source)
> >  > > > at java.util.concurrent.FutureTask.run(Unknown Source)
> >  > > > at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown
> >  > > > Source)
> >  > > > at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown
> >  > > > Source)
> >  > > > at java.lang.Thread.run(Unknown Source)
> >  > > >
> >  > > > My mkgmap call is:
> >  > > > java -jar mkgmap.jar --max-jobs=4
> >  > > > --createboundsfile=africa-boundaries.osm.gz
> > --bounds=bounds/africa *.osm
> >  > > >
> >  > > > WanMil
> >  > > >
> >  > > > > Hi WanMil,
> >  > > > >
> >  > > > > sorry, one of my last patches for the performance branch corrupted
> >  > > > > BoundaryUtil.
> >  > > > > It was no longer able to read the legacy *.bnd format (also created
> >  > > in the
> >  > > > > preparer) :-(
> >  > > > >
> >  > > > > Attached is the patch.
> >  > > > >
> >  > > > > Gerd
> >  > > > >
> >  > > > >
> > http://gis.19327.n5.nabble.com/file/n5554370/BoundaryUtil.java.patch
> >  > > > > BoundaryUtil.java.patch
> >  > > > >
> >  > > > > --
> >  > > > > View this message in context:
> >  > >
> > http://gis.19327.n5.nabble.com/Patch-Error-in-BoundaryUtil-tp5554370p5554370.html
> >  > > > > Sent from the Mkgmap Development mailing list archive at
> > Nabble.com.
> >  > > > > _______________________________________________
> >  > > > > mkgmap-dev mailing list
> >  > > > > mkgmap-dev at lists.mkgmap.org.uk
> >  > > > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >  > > >
> >  > > > _______________________________________________
> >  > > > mkgmap-dev mailing list
> >  > > > mkgmap-dev at lists.mkgmap.org.uk
> >  > > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >  > >
> >  > >
> >  > > _______________________________________________
> >  > > mkgmap-dev mailing list
> >  > > mkgmap-dev at lists.mkgmap.org.uk
> >  > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >  >
> >  > _______________________________________________
> >  > mkgmap-dev mailing list
> >  > mkgmap-dev at lists.mkgmap.org.uk
> >  > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >
> >
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev at lists.mkgmap.org.uk
> > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> 
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20120312/840355ec/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: BoundaryUtil_v3.patch
Type: application/x-download
Size: 14139 bytes
Desc: not available
Url : http://lists.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20120312/840355ec/attachment.bin 