Re: [mkgmap-dev] Splitter PBF support patch
Ok, take 2 on the splitter PBF patch. All new files should be present. I also fixed the EOL mode, so the diffs should be better. I don't have commit access and not looking for it at this point. To enable the PBF output, I added the option --pbf. If omitted, it should output the normal .osm.gz format.
On Thu, May 19, 2011 at 03:37:19PM -0400, Francisco Moraes wrote:
Ok, take 2 on the splitter PBF patch. All new files should be present. I also fixed the EOL mode, so the diffs should be better.
That would explain why the new diff is shorter. I guess we should always "svn propset svn:eol-style native" on all non-binary files.
To enable the PBF output, I added the option --pbf. If omitted, it should output the normal .osm.gz format.
Doh, I missed this. I had the broken *.osm.pbf files lying around from a previous broken run, and was just about to complain that the patch fails to close the polygons, as your initial splitter.jar release did. I will try this one more time on today's finland.osm.pbf, but I will report the results tomorrow. Marko
On Thu, May 19, 2011 at 11:46:57PM +0300, Marko Mäkelä wrote:
Doh, I missed this. I had the broken *.osm.pbf files lying around from a previous broken run, and was just about to complain that the patch fails to close the polygons, as your initial splitter.jar release did. I will try this one more time on today's finland.osm.pbf
The split time was reduced from 140ish (IIRC) to 102 seconds. There seems to be something wrong with the pbf output, because mkgmap is outputting this kind of messages: 2011/05/19 23:53:11 WARNING (OsmBinHandler): 63240005.osm.pbf: Way http://www.openstreetmap.org/browse/way/107168753 references undefined node 1231941341 2011/05/19 23:53:11 WARNING (OsmBinHandler): 63240005.osm.pbf: Way http://www.openstreetmap.org/browse/way/107168753 references undefined node 1232127462 2011/05/19 23:53:11 WARNING (OsmBinHandler): 63240005.osm.pbf: Way http://www.openstreetmap.org/browse/way/107168753 references undefined node 1232017145 Could it be that when you delete nodes outside the output tile, you forget to delete the node references from the way? I have not seen these messages before. Or is the osm.gz output of the splitter equally broken, but the mkgmap XML parser is not complaining about ways referencing deleted nodes? Apart from these warnings, I am still getting the warnings about unclosed polygons, like this: 2011/05/19 23:54:39 WARNING (StyledConverter): 63240008.osm.pbf: Unclosed way http://www.openstreetmap.org/browse/way/23679990 [natural=water] should be converted as shape but the start or end point lies inside the bbox. Skip it. For some reason, the pbf splitter workflow failed to issue the data error (one dead-end oneway) that I got reported for the same finland.osm.pbf when splitting it to osm.gz. Can you fix these issues? Marko
On Fri, May 20, 2011 at 12:00:58AM +0300, Marko Mäkelä wrote:
There seems to be something wrong with the pbf output, because mkgmap is outputting this kind of messages:
2011/05/19 23:53:11 WARNING (OsmBinHandler): 63240005.osm.pbf: Way http://www.openstreetmap.org/browse/way/107168753 references undefined node 1231941341
This turns out to be a chattiness difference in the mkgmap parsers. The XML parser does not seem to complain when ways are referencing undefined nodes. I tested by checking the nodes of way 57746082, the role=outer way of relation 900367, in my tile 63240006.osm.gz. I searched for '2442732[01345789][0-9]' and did not find any matches apart from the nd ref in the way.
Apart from these warnings, I am still getting the warnings about unclosed polygons, like this:
2011/05/19 23:54:39 WARNING (StyledConverter): 63240008.osm.pbf: Unclosed way http://www.openstreetmap.org/browse/way/23679990 [natural=water] should be converted as shape but the start or end point lies inside the bbox. Skip it.
For some reason, the pbf splitter workflow failed to issue the data error (one dead-end oneway) that I got reported for the same finland.osm.pbf when splitting it to osm.gz.
These two issues are still unexplained and need to be fixed in my opinion. Best regards, Marko
On 2:59 PM, Marko Mäkelä wrote:
Apart from these warnings, I am still getting the warnings about unclosed polygons, like this:
2011/05/19 23:54:39 WARNING (StyledConverter): 63240008.osm.pbf: Unclosed way http://www.openstreetmap.org/browse/way/23679990 [natural=water] should be converted as shape but the start or end point lies inside the bbox. Skip it.
For some reason, the pbf splitter workflow failed to issue the data error (one dead-end oneway) that I got reported for the same finland.osm.pbf when splitting it to osm.gz.
These two issues are still unexplained and need to be fixed in my opinion. So, are you saying that the splitter itself should have done something different on that tile with that way? I briefly compared both the XML and PBF->XML output and they seemed to both have the same node refs.
Francisco
On Wed, May 25, 2011 at 01:36:07PM -0400, Francisco Moraes wrote:
On 2:59 PM, Marko Mäkelä wrote:
Apart from these warnings, I am still getting the warnings about unclosed polygons, like this:
2011/05/19 23:54:39 WARNING (StyledConverter): 63240008.osm.pbf: Unclosed way http://www.openstreetmap.org/browse/way/23679990 [natural=water] should be converted as shape but the start or end point lies inside the bbox. Skip it.
For some reason, the pbf splitter workflow failed to issue the data error (one dead-end oneway) that I got reported for the same finland.osm.pbf when splitting it to osm.gz.
These two issues are still unexplained and need to be fixed in my opinion. So, are you saying that the splitter itself should have done something different on that tile with that way? I briefly compared both the XML and PBF->XML output and they seemed to both have the same node refs.
Yes, I suspect that the PBF splitter output or the mkgmap PBF parser is broken or different from the XML output or parser in this aspect. I fed the same finland.osm.pbf file to the toolchain. The only difference was the output format of splitter (osm.pbf instead of osm.gz). As far as I understand, it is a little challenging to compare the XML and PBF with each other, because splitter outputs 0.5 XML, which is no longer supported by Osmosis. I may look at the code in some days if needed. Sorry, I get an overdosis of software development at work, and I try to avoid doing too much of it in my spare time. Best regards, Marko
Hi I compared the pbf and xml formats as output from a small file which only produced one output tile. I converted the pbf to xml with osmosis. Its tricky to compare them, but by counting elements it seemed that the main differences was that there were fewer relations in the pbf output. Further more it just seemed to be the final few that were missing. Comparing with the osmosis code I think there should be a switchTypes() call to ensure that the last relations are written out (see attached patch). After the patch, I can't see any further substantial difference between the outputs. Compiled img files are the same size although not entirely identical. I'm not sure if the differences are significant or not yet. There could still be bugs in the mkgmap pbf reader code. Although it has been in for a while, its probably not been used much until now. ..Steve
On Fri, May 27, 2011 at 11:29:15AM +0100, Steve Ratcliffe wrote:
There could still be bugs in the mkgmap pbf reader code. Although it has been in for a while, its probably not been used much until now.
I routinely use the PBF input format for creating my --style=routes-* layers. If there are bugs in the PBF input, they could be in the parsing of polygons. I guess I could find this out easily: create a style that translates only natural=water polygons, and feed finland.osm.pbf to it. If it complains about areas that the default style does not complain about, then the PBF parser is to blame. Some time ago, I complained that the PBF code path did not produce some warnings about oneways coming from or going to nowhere, while the XML path did. I just realized that the head of the mkgmap.log.* was most likely discarded because of the numerous warnings about unreferenced nodes in ways. I only had configured 4 log files of at most 5MB each. Marko
On Fri, May 27, 2011 at 06:36:32PM +0300, Marko Mäkelä wrote:
If there are bugs in the PBF input, they could be in the parsing of polygons. I guess I could find this out easily: create a style that translates only natural=water polygons, and feed finland.osm.pbf to it. If it complains about areas that the default style does not complain about, then the PBF parser is to blame.
The result is somewhat inconclusive. I copied the default style, discarded all points, lines and relations rules, and kept only the natural=* polygons rules. The map generation was finally interrupted by this: uk.me.parabola.imgfmt.MapFailedException: There is not enough room in a single garmin map for all the input data Not much error log was produced. All messages about unclosed polygons seemed legitimate. There were 104 such messages. 101 of them were on my blacklist (polygons severed by Geofabrik's cutting polygon). It seems to me that the mkgmap PBF parser works flawlessly. Marko
Hi Steve, On Fri, May 27, 2011 at 11:29:15AM +0100, Steve Ratcliffe wrote:
There could still be bugs in the mkgmap pbf reader code. Although it has been in for a while, its probably not been used much until now.
The attached patch zaps the mkgmap warnings about unreferenced nodes. With your patch applied on top of the splitter-pbf branch, I got exactly the same warnings through my filter that I got through the XML toolchain today. The gmapsupp.img size is identical, and the warnings about unclosed polygons are gone. Can you please commit your patch to the splitter-pbf branch? Is it OK to commit the warning-zapping patch to mkgmap, or would it be feasible to do something similar to Osmosis --used-node in splitter? Marko
Hi Steve, Thanks for committing the patches. Is there a downloadable .jar of splitter-pbf-write r175 available somewhere for download, or would it be possible to merge the pbf-write branch to trunk now? I have built the map a few times with it, and have not experienced anything unusual. Best regards, Marko
On 09/06/11 07:02, Marko Mäkelä wrote:
Hi Steve,
Thanks for committing the patches.
No problem.
Is there a downloadable .jar of splitter-pbf-write r175 available somewhere for download, or would it be possible to merge the pbf-write branch to trunk now? I have built the map a few times with it, and have not experienced anything unusual.
I've put one here URL: http://files.mkgmap.org.uk/download/28/splitter-r175.zip Description: Splitter with the pbf write patch (by Francisco Moraes). But I do think it is ready to merge, I am sure that it now gives results identical to XML. The only thing is changing the option name. ..Steve
Here's the latest patch with the output type option instead of --pbf. Use --output=pbf to select PBF output or omit it or specify --output=xml to use the old XML output. Default is still XML.
Here's the latest patch with the output type option instead of --pbf. Use --output=pbf to select PBF output or omit it or specify --output=xml to use the old XML output. Default is still XML.
Francisco, thanks a lot for your splitter pbf support. That's great! Just my two cents: I would like to have pbf enabled as default because it is the 'better' format regarding speed and disk usage. To avoid problems a hint could be added to the download page. WanMil
Hi
Ok, take 2 on the splitter PBF patch. All new files should be present. I also fixed the EOL mode, so the diffs should be better. I don't have commit access and not looking for it at this point.
I have committed this on a new pbf-write branch. I've left out the removal of the @Override annotations however. I'm not against removing them (we don't use them in mkgmap) but we should do it as a separate checkin if that is what we decide to do. I'm also in favour of making the default output type PBF, or at the very least having the output type be the same as the input type. Thanks ..Steve
Hi
Ok, take 2 on the splitter PBF patch. All new files should be present. I also fixed the EOL mode, so the diffs should be better. I don't have commit access and not looking for it at this point.
I have committed this on a new pbf-write branch. I've left out the removal of the @Override annotations however.
I'm not against removing them (we don't use them in mkgmap) but we should do it as a separate checkin if that is what we decide to do.
I'm also in favour of making the default output type PBF, or at the very least having the output type be the same as the input type.
+1 for pbf as default output type WanMil
Thanks
..Steve
Am 23.05.2011 um 18:32 schrieb WanMil:
Hi
Ok, take 2 on the splitter PBF patch. All new files should be present. I also fixed the EOL mode, so the diffs should be better. I don't have commit access and not looking for it at this point.
I have committed this on a new pbf-write branch. I've left out the removal of the @Override annotations however.
I'm not against removing them (we don't use them in mkgmap) but we should do it as a separate checkin if that is what we decide to do.
I'm also in favour of making the default output type PBF, or at the very least having the output type be the same as the input type.
+1 for pbf as default output type
WanMil
Also +1 for pbf as default output type Short question: Can the locatorbranch handle pbf-files? Martin
Thanks
..Steve
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Push push. Still not commited... Would be nice if the splitter trunk would get the PBF support implemented. I used: http://files.mkgmap.org.uk/download/28/splitter-r175.zip without any probs for my latest round of map updates (and due to my overly extensive style, I'm usually one of the first to run into bugs). Also +1 for making pbf the default output. On 24.05.2011 12:58, Martin wrote:
Am 23.05.2011 um 18:32 schrieb WanMil:
Hi
Ok, take 2 on the splitter PBF patch. All new files should be present. I also fixed the EOL mode, so the diffs should be better. I don't have commit access and not looking for it at this point. I have committed this on a new pbf-write branch. I've left out the removal of the @Override annotations however.
I'm not against removing them (we don't use them in mkgmap) but we should do it as a separate checkin if that is what we decide to do.
I'm also in favour of making the default output type PBF, or at the very least having the output type be the same as the input type. +1 for pbf as default output type
WanMil Also +1 for pbf as default output type
Short question: Can the locatorbranch handle pbf-files?
Martin
Thanks
..Steve
mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
On 2:59 PM, Steve Ratcliffe wrote:
I'm not against removing them (we don't use them in mkgmap) but we should do it as a separate checkin if that is what we decide to do. Well, the only reason I removed them is that Eclipse will not compile without. As per Javadoc, it is an error to place @Override on a method that's not overriding a base method.
A question on general splitter operation: is it supposed to remove node references from ways if the node is outside the area bounding box?
Hi
Well, the only reason I removed them is that Eclipse will not compile without. As per Javadoc, it is an error to place @Override on a method that's not overriding a base method.
That is a difference between java 1.5 and 1.6. In 1.5 it was invalid to place @Override on method which is implementing a method of an interface, whereas in 1.6 it is allowed. ..Steve
On Tue, May 24, 2011 at 03:36:31PM -0400, Francisco Moraes wrote:
A question on general splitter operation: is it supposed to remove node references from ways if the node is outside the area bounding box?
All I can tell is that Osmosis does so. If it is too hard to remove the references to omitted nodes from the ways, I guess that we could tone down the mkgmap parser, so that it does not complain about this when reading osm.pbf. The XML parser already does not complain when reading. I tested the splitter osm.pbf output again on yesterday's dump. The resulting img files were smaller than the ones produced from the osm.gz output. Thus, I would believe that there is something wrong with the splitter osm.pbf output that causes mkgmap to think that polygons are no longer closed and must be omitted. Best regards, Marko
participants (6)
-
Felix Hartmann -
Francisco Moraes -
Marko Mäkelä -
Martin -
Steve Ratcliffe -
WanMil