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

Text Detection: add ppocr-v2 detect -WIP #66

Closed
wants to merge 8 commits into from

Conversation

the-star-sea
Copy link
Contributor

  • add demo
  • add example in readme

@zihaomu zihaomu self-assigned this Jun 29, 2022
@zihaomu zihaomu added the GSoC Google Summer of Code projected related label Jun 29, 2022
@zihaomu zihaomu changed the title add ppocr-v2 detect add ppocr-v2 detect -WIP Jun 29, 2022
@fengyuentau fengyuentau added the add model request to add a new model label Jul 13, 2022
Copy link
Member

@zihaomu zihaomu left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
Newly added models do not need the old copyright.

models/text_detection_ppdetect/demo.py Outdated Show resolved Hide resolved
models/text_detection_ppdetect/ppdetect.py Outdated Show resolved Hide resolved
@zihaomu
Copy link
Member

zihaomu commented Jul 20, 2022

Hi @the-star-sea. Since in OpenCV the DB has been supported by High-Level API, can you provide more speed and accuracy test data for DB and proposed ppocr?

@zihaomu zihaomu changed the title add ppocr-v2 detect -WIP OCR Detection: add ppocr-v2 detect -WIP Jul 20, 2022
@zihaomu zihaomu changed the title OCR Detection: add ppocr-v2 detect -WIP Text Detection: add ppocr-v2 detect -WIP Jul 20, 2022
@the-star-sea
Copy link
Contributor Author

Hi @the-star-sea. Since in OpenCV the DB has been supported by High-Level API, can you provide more speed and accuracy test data for DB and proposed ppocr?

ok.I will do it

@the-star-sea
Copy link
Contributor Author

image.png

@zihaomu
Copy link
Member

zihaomu commented Jul 25, 2022

@the-star-sea the ppocr's speed is super fast.
two questions:

  1. Are the pre-process and the post-process of ppocr's the same as the DB net?
  2. Is there accuracy test result?

@the-star-sea
Copy link
Contributor Author

@the-star-sea the ppocr's speed is super fast. two questions:

  1. Are the pre-process and the post-process of ppocr's the same as the DB net?
  2. Is there accuracy test result?

1.almost the same
2.
image.png
image.png

@zihaomu
Copy link
Member

zihaomu commented Jul 25, 2022

Thank you @the-star-sea! The accuracy result is good. Which validation data are used? How many data it has?

@the-star-sea
Copy link
Contributor Author

icdar2015.500 imgs.

@zihaomu
Copy link
Member

zihaomu commented Jul 25, 2022

Thanks for the clarification. The DB model has hidded the post-process in the high level api, some thing like the following:

model = cv.dnn_TextDetectionModel_DB(
            cv.dnn.readNet(self._modelPath)
        )

# time start
model.detect(image)
# time stop

In order to get a fair speed comparison result, how did you test the speed of ppocr?

@the-star-sea
Copy link
Contributor Author

I write the config file in benchmark and add function support in ppdetect.It just tests the speed of infering onnx model.
Besides.the onnx filesize of ppdetect is 2284 while DB is 47628.

@zihaomu
Copy link
Member

zihaomu commented Jul 25, 2022

47628

That's a great answer. Thanks!
Can we re-use the High-Level API of DB for PPocr with no change or little change?

model = cv.dnn_TextDetectionModel_DB(
            cv.dnn.readNet(modelPath_ppocr)
        )
model.detect(image)

@the-star-sea
Copy link
Contributor Author

I think there need some changes because ppocr doesnot need polygon threhold.I will do it.

@zihaomu
Copy link
Member

zihaomu commented Aug 30, 2022

@the-star-sea Please keep one in the mobile and normal models, as we discussed before, to avoid confusing users.

@the-star-sea
Copy link
Contributor Author

@the-star-sea Please keep one in the mobile and normal models, as we discussed before, to avoid confusing users.

ok.I will do it

Copy link
Member

@zihaomu zihaomu left a comment

Choose a reason for hiding this comment

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

LGTM!


Real-time Scene Text Detection with Differentiable Binarization

This model is ported from [PaddleOCR](https://github.com/PaddlePaddle/PaddleOCR).
Copy link
Member

Choose a reason for hiding this comment

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

How about adding the original model link here?

Copy link
Member

@fengyuentau fengyuentau left a comment

Choose a reason for hiding this comment

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

I suggest we remove DB from opencv_zoo after this pull request is merged, make a new high level api specifically for this model in opencv and update the wrapper class.

models/__init__.py Show resolved Hide resolved
@@ -37,3 +37,4 @@ def register(self, item):
MODELS.register(MobileNetV2)
MODELS.register(MPPalmDet)
MODELS.register(LPD_YuNet)
MODELS.register(PPDetect)
Copy link
Member

Choose a reason for hiding this comment

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

EOL before EOF

@zihaomu
Copy link
Member

zihaomu commented Sep 5, 2022

I suggest we remove DB from opencv_zoo after this pull request is merged.

Hi @fengyuentau, how about leaving it to the next PR to remove DB model, because we have a lot of speed tests based on the DB model. In addition, the high-level API based ppocr model has not been completed and is expected to be completed by the end of September.

@fengyuentau
Copy link
Member

Sure, thats exactly what I meant. Just want to emphasize this plan.

@zihaomu zihaomu requested a review from fengyuentau September 8, 2022 00:29
@fengyuentau
Copy link
Member

@zihaomu Please update benchmark results on this model.

@zihaomu
Copy link
Member

zihaomu commented Sep 8, 2022

@zihaomu Please update benchmark results on this model.

Hi, @fengyuentau the benchmark results will be updated at PR #73, the student will complete it in the near future.

@fengyuentau
Copy link
Member

When we add or update a model in opencv zoo, we always update the average forward latency in the same pull request.

@zihaomu
Copy link
Member

zihaomu commented Sep 8, 2022

Ok, let's wait.

@fengyuentau
Copy link
Member

We dont need to wait for #73 as it is for accuracy. At least we need the speed of this model to merge a pull request of adding or updating a model.

@zihaomu
Copy link
Member

zihaomu commented Oct 13, 2022

After this PR is merged, PP-OCR_v3 can be supported and loaded with high-level API. And I think we can only put PP-OCR_v3 in opencv_zoo since it has better accuracy.

help_msg_backends += "; {:d}: TIMVX"
help_msg_targets += "; {:d}: NPU"
except:
print('This version of OpenCV does not support TIM-VX and NPU. Visit https://gist.github.com/fengyuentau/5a7a5ba36328f2b763aea026c43fa45f for more information.')

Choose a reason for hiding this comment

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

It looks very strange that sample proposes to use external gist, bug not official documentation or wiki. I propose to use https://github.com/opencv/opencv/wiki/TIM-VX-Backend-For-Running-OpenCV-On-NPU. @fengyuentau Please extend wiki page if something is missing there.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the review. IIRC, this link was added before we have the wiki in opencv. Will update this in a separate pull request.

@fengyuentau
Copy link
Member

fengyuentau commented Oct 21, 2022

@zihaomu Anything else is blocking this PR from merge?

Update: we need to merge with benchmark results. Could you run benchmarking with this model? @zihaomu

@zihaomu
Copy link
Member

zihaomu commented Oct 21, 2022

@zihaomu Anything else is blocking this PR from merge?

Update: we need to merge with benchmark results. Could you run benchmarking with this model? @zihaomu

This PR should be closed after the OpenCV support new DB API, since new DB API support pp-ocr V3 and V2.

@fengyuentau
Copy link
Member

This PR should be closed

Didn't we agree on adding this model (pp-ocr) in place of db? Why are we closing this PR?

@zihaomu
Copy link
Member

zihaomu commented Oct 25, 2022

We can directly support ppocr-DB v2 and v3 at new API.

@zihaomu zihaomu closed this Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add model request to add a new model GSoC Google Summer of Code projected related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants