Add ALTO Output Functionality #2067

jakesebright · 2018-11-20T23:48:46Z

I have added support for ALTO output as described in this issue.

ALTO XML can be output by using the config file at tessdata/configs/alto. I have confirmed that this output validates against the schema defined here.

This is my first pull request to a code base of this size, so please have patience if I have misunderstood anything. I use ALTO quite often and would love to have support for it in tesseract. I would be happy to make additional recommended improvements to my implementation.

amitdo · 2018-11-21T09:33:02Z

@zdenop , @stweil

Should the alto renderer code be in its own file like the pdf renderer or in baseapi.cpp?

Another option is to put all text renderers in one file textrenderers.cpp.

stweil

Thank you very much for this contribution. I'll test it for our images later this week.

Up to now I only reviewed the code and have added several inline remarks.

One general remark: personally I like things sorted, be it new code blocks, several lines of code or other things, either alphabetically or some other criteria. That helps when maintaining the code and also gives nicer help texts, for example. So I suggest to add new code not always at the end.

stweil · 2018-11-21T09:52:56Z

src/api/altorenderer.cpp

+                     "\t\t<OCRProcessing ID=\"OCR_0\">\n"
+                     "\t\t\t<ocrProcessingStep>\n"
+                     "\t\t\t\t<processingSoftware>\n"
+                     "\t\t\t\t\t<softwareName>tesseract 4.0.0</softwareName>\n"


The version should be taken from TessBaseAPI::Version() here.

stweil · 2018-11-21T09:57:12Z

src/api/altorenderer.cpp

+    }
+
+    /**
+    * Append the ALTO XML for the end of the document


Would C++ (///) comments be better? Much of the Tesseract code was written before using C++, so is not necessarily a good template.

https://google.github.io/styleguide/cppguide.html#Function_Comments

stweil · 2018-11-21T09:58:47Z

src/api/altorenderer.cpp

+            // convert input name from ANSI encoding to utf-8
+          int str16_len =
+              MultiByteToWideChar(CP_ACP, 0, input_file_->string(), -1, nullptr, 0);
+          wchar_t *uni16_str = new WCHAR[str16_len];


Use std::vector here.

I need help making this change. I did not write this section of code (It came from the hOCR implementation), so I am not 100% sure of what it is doing.

stweil · 2018-11-21T10:02:53Z

src/api/altorenderer.cpp

+          char *utf8_str = new char[utf8_len];
+          WideCharToMultiByte(CP_UTF8, 0, uni16_str, str16_len, utf8_str,
+                              utf8_len, nullptr, nullptr);
+          *input_file_ = utf8_str;


Could *input_file be used directly instead of utf8_str thus avoiding the copying and delete[]?

I need help making this change. I did not write this section of code (It came from the hOCR implementation.), so I am not 100% sure of what it is doing.

stweil · 2018-11-21T10:04:40Z

src/api/altorenderer.cpp

+    /**
+    * Append the ALTO XML for the layout of the image
+    */
+    bool TessAltoRenderer::AddImageHandler(TessBaseAPI *api) {


The recommended style for new or modified code is TessBaseAPI* api.

stweil · 2018-11-21T10:06:02Z

src/api/altorenderer.cpp

+            return ret;
+        }
+
+    }


Missing line feed as last character of the source file.

Suggested change

}

}

stweil · 2018-11-21T10:07:59Z

src/api/baseapi.cpp

@@ -94,34 +94,34 @@ BOOL_VAR(stream_filelist, FALSE, "Stream a filelist from stdin");
 namespace tesseract {

 /** Minimum sensible image size to be worth running tesseract. */
-const int kMinRectSize = 10;
+    const int kMinRectSize = 10;


Here are several code changes which are unrelated to ALTO.

stweil · 2018-11-21T10:11:04Z

src/api/capi.cpp

@@ -66,6 +66,11 @@ TESS_API TessResultRenderer* TESS_CALL TessHOcrRendererCreate2(const char* outpu
    return new TessHOcrRenderer(outputbase, font_info);
 }

+TESS_API TessResultRenderer* TESS_CALL TessAltoRendererCreate(const char* outputbase)
+{
+    return new TessHOcrRenderer(outputbase);


Copy+paste error. :-)

Suggested change

return new TessHOcrRenderer(outputbase);

return new TessAltoRenderer(outputbase);

stweil · 2018-11-21T10:20:21Z

Should the alto renderer code be in its own file like the pdf renderer or in baseapi.cpp?

Personally I think that an own file is good here. What would be the advantages of the alternatives?

stweil · 2018-11-21T10:21:44Z

src/api/altorenderer.cpp

+    * Append the ALTO XML for the layout of the image
+    */
+    bool TessAltoRenderer::AddImageHandler(TessBaseAPI *api) {
+        const std::unique_ptr<const char[]> hocr(api->GetAltoText(imagenum()));


That requires #include <memory.h> (see build failures from continuous integration).

I'm sorry, my comment was wrong. It should be just #include <memory>.

zdenop · 2018-11-21T10:57:19Z

@amitdo : see @jbreiden comment in #419 (comment)

amitdo · 2018-11-21T12:27:13Z

Ok, alto renderer will get its own file.

What about the other renderers, should each of them get its own file?

stweil · 2018-11-21T13:15:02Z

src/api/baseapi.cpp

@@ -118,10 +118,10 @@ namespace tesseract {
    static void addAvailableLanguages(const STRING &datadir, const STRING &base,
                                      GenericVector<STRING>* langs)
    {
-      const STRING base2 = (base.string()[0] == '\0') ? base : base + "/";
-      const size_t extlen = sizeof(kTrainedDataSuffix);
+        const STRING base2 = (base.string()[0] == '\0') ? base : base + "/";


You can use C++ comments, and Tesseract uses indentation steps of 2, so at least most of this commit is not needed or even not wanted at all.

jakesebright · 2018-11-22T15:27:23Z

Thank you very much for these comments and suggestions. They are incredibly helpful and informative. I will work on making these improvements over the next few days.

A note: I had originally implemented ALTO functionality in api/baseapi.cpp but then separated it out due to the aforementioned comment.

stweil · 2018-11-22T17:05:49Z

@jakesebright, I have now processed a page with your code. It can be seen here. Full text view must be activated by clicking on the 4th icon in the top menu.

Obviously the DFG viewer software which is used there has problems with the separation of the words. All other pages which were made by ABBYY Finereader don't have that problem because it separates the <String> tags by <SP> tags.

All ALTO files are available online.

stweil · 2018-11-22T20:55:30Z

tessdata/configs/Makefile.am also needs an update to install the configuration file tessdata/configs/alto for ALTO.

stweil · 2018-11-28T14:15:20Z

@jakesebright, do you plan to add more commits, for example to fix the build error? Or should we take your contribution as it is and fix the remaining issues on our side?

jakesebright · 2018-11-29T04:18:33Z

@stweil Yes, I should have time to work on this tomorrow. Thank you for following up and for your patience.

jakesebright · 2018-11-30T03:21:28Z

I have made all of the suggested changes except for those where I left a comment noting otherwise.

I am still getting build failures from the continuous integration, and am having a hard time figuring out why this is happening. Could anyone offer some help / advice with this failure?

jbreiden · 2018-11-30T03:41:14Z

I just want to say thank you very much for adding this feature. At one point I reasonably understood the HOCR renderer so l will try to take a look when I find time, if you are still stuck.

stweil · 2018-11-30T04:37:55Z

src/api/altorenderer.cpp

+// limitations under the License.
+
+#include "baseapi.h"
+#include <memory.h>


Suggested change

#include <memory.h>

#include <memory> // for unique_ptr

stweil · 2018-11-30T04:41:27Z

src/api/altorenderer.cpp

+    bool TessAltoRenderer::BeginDocumentHandler() {
+        AppendString(
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+        "<alto xmlns=\"http://www.loc.gov/standards/alto/ns-v3#\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:schemaLocation=\"http://www.loc.gov/standards/alto/ns-v3# http://www.loc.gov/alto/v3/alto-3-0.xsd\">\n"


Suggested change

"<alto xmlns=\"http://www.loc.gov/standards/alto/ns-v3#\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:schemaLocation=\"http://www.loc.gov/standards/alto/ns-v3# http://www.loc.gov/alto/v3/alto-3-0.xsd\">\n"

"<alto xmlns=\"http://www.loc.gov/standards/alto/ns-v3#\" "

"xmlns:xlink=\"http://www.w3.org/1999/xlink\" "

"xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" "

"xsi:schemaLocation=\"http://www.loc.gov/standards/alto/ns-v3# http://www.loc.gov/alto/v3/alto-3-0.xsd\">\n"

stweil · 2018-11-30T04:50:52Z

@jakesebright, thank you for the updates and all of you work. The build failure is still there because I gave a wrong advice: the missing include file is memory, not memory.h. I fixed it now.

stweil · 2018-12-12T20:12:29Z

src/api/altorenderer.cpp

+            alto_str += "\" HEIGHT=\"";
+            alto_str.add_str_int("", rect_height_);
+            alto_str += "\" PHYSICAL_IMG_NR=\"";
+            alto_str.add_str_int("", rect_height_);


@jakesebright , rect_height does not look like a PHYSICAL_IMG_NR. Is this a copy+paste error? What was intended here?

Yes, this is a copy/paste error. I believe it should be page_number instead of rect_height.

I have a fix in pull request #2122.

jakesebright added 3 commits November 19, 2018 20:21

Add support for ALTO output

e38b984

Separate ALTO functionality into altorenderer.cpp

38b705c

Clean up formatting

3ba5a09

stweil requested changes Nov 21, 2018

View reviewed changes

stweil reviewed Nov 21, 2018

View reviewed changes

stweil mentioned this pull request Nov 22, 2018

ALTO output: Missing <SP> tags between <String> tags UB-Mannheim/ocr-fileformat#78

Open

Remove changes unrelated to ALTO

2a91379

stweil added the feature request label Nov 28, 2018

jakesebright added 4 commits November 29, 2018 21:11

Add #include <memory.h> to altorenderer.cpp

664f7ea

Small style fixes

a74b6b1

Dynamically generate tesseract version value

7da328e

Re-organize functions alphabetically

5c8cf13

stweil reviewed Nov 30, 2018

View reviewed changes

Fix include statement

62862e4

ghost assigned stweil Nov 30, 2018

ghost added the review label Nov 30, 2018

stweil approved these changes Nov 30, 2018

View reviewed changes

stweil merged commit d7cee03 into tesseract-ocr:master Nov 30, 2018

ghost removed the review label Nov 30, 2018

jakesebright deleted the alto branch December 1, 2018 23:48

stweil reviewed Dec 12, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ALTO Output Functionality #2067

Add ALTO Output Functionality #2067

jakesebright commented Nov 20, 2018

amitdo commented Nov 21, 2018 •

edited

Loading

stweil left a comment

stweil Nov 21, 2018

stweil Nov 21, 2018

amitdo Nov 21, 2018

stweil Nov 21, 2018

jakesebright Nov 30, 2018

stweil Nov 21, 2018

jakesebright Nov 30, 2018

stweil Nov 21, 2018

stweil Nov 21, 2018

stweil Nov 24, 2018

stweil Nov 21, 2018

stweil Nov 21, 2018

stweil Nov 24, 2018

stweil commented Nov 21, 2018

stweil Nov 21, 2018

stweil Nov 30, 2018

zdenop commented Nov 21, 2018

amitdo commented Nov 21, 2018

stweil Nov 21, 2018

jakesebright commented Nov 22, 2018

stweil commented Nov 22, 2018 •

edited

Loading

stweil commented Nov 22, 2018

stweil commented Nov 28, 2018

jakesebright commented Nov 29, 2018

jakesebright commented Nov 30, 2018

jbreiden commented Nov 30, 2018 via email

stweil Nov 30, 2018

stweil Nov 30, 2018

stweil commented Nov 30, 2018 •

edited

Loading

stweil Dec 12, 2018

jakesebright Dec 14, 2018

stweil Dec 16, 2018

	return new TessHOcrRenderer(outputbase);
	return new TessAltoRenderer(outputbase);

-        "<alto xmlns=\"http://www.loc.gov/standards/alto/ns-v3#\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:schemaLocation=\"http://www.loc.gov/standards/alto/ns-v3# http://www.loc.gov/alto/v3/alto-3-0.xsd\">\n"
+        "<alto xmlns=\"http://www.loc.gov/standards/alto/ns-v3#\" "
+        "xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
+        "xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" "
+        "xsi:schemaLocation=\"http://www.loc.gov/standards/alto/ns-v3# http://www.loc.gov/alto/v3/alto-3-0.xsd\">\n"

Add ALTO Output Functionality #2067

Add ALTO Output Functionality #2067

Conversation

jakesebright commented Nov 20, 2018

amitdo commented Nov 21, 2018 • edited Loading

stweil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stweil commented Nov 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zdenop commented Nov 21, 2018

amitdo commented Nov 21, 2018

Choose a reason for hiding this comment

jakesebright commented Nov 22, 2018

stweil commented Nov 22, 2018 • edited Loading

stweil commented Nov 22, 2018

stweil commented Nov 28, 2018

jakesebright commented Nov 29, 2018

jakesebright commented Nov 30, 2018

jbreiden commented Nov 30, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stweil commented Nov 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitdo commented Nov 21, 2018 •

edited

Loading

stweil commented Nov 22, 2018 •

edited

Loading

stweil commented Nov 30, 2018 •

edited

Loading