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

Add OCR acction with tessercat #3074

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rsrdesarrollo
Copy link

@rsrdesarrollo rsrdesarrollo commented Jan 27, 2023

This PR adds an OCR action using tesseract. Fix #702

It needs tesseract to be in path or configure full path

image

image

@mmahmoudian
Copy link
Member

mmahmoudian commented Jan 28, 2023

Thanks for the PR, but please always discuss the new features before spending time implementing it. Also it would be faster for you if you search among the open and closed issues to see if a topic is discussed.

In my opinion this PR is unacceptable for few reasons:

  1. OCR is out of the scope of a screenshot tool, but it is already possible to do it as explained in Easy OCR #1344
  2. We will only allow such features via our plugin system which is explained in details in Plugin RFC #2529

I suggest we keep your PR (at least partially) for when the plugin system is implemented, as some of the parts (e.g icons) can be used in the PR.

I would also like to know the opinion of other devs (e.g @veracioux , @hosiet , @ZetaoYang ) as the above does not necessarily reflect the stance of the project as a team, but rather just my opinion.

@rsrdesarrollo
Copy link
Author

To be completely honest, the implementing time was nearly none, I needed the feature, and I'm currently using it in a more effective and usable way than piping commands.

Also, this project, despite being an amazing tool, lacks of any OCR capability and, for the past nearly 3 years, since the first request to implement this came (#720) AFAIK no one has done really any code to implement the feature in a usable way for any kind of user. AFAIK plugins system is only a concept, nothing implemented.

OCR is out of the scope of a screenshot tool,

Let me, greenshot, sharex and snagit disagree, OCR is indeed a great and wanted feature for screenshot tools.

In my humble opinion, here is a PR to allow 90% of your users to do what they need today. You can easily think about moving it to plugins when the plugin system is implemented.

That being said, I have no need for this PR to be merged, I can live with my fork, just wanted to give something back to the community, I guess anyone who needs it can take it from my copy of the project.

Regards!

@debuglevel

This comment was marked as off-topic.

@erfanium
Copy link

erfanium commented Apr 21, 2024

OCR is out of the scope of a screenshot tool

I think this argument can also be applied to the currently implemented post-processing (draw shape, pen, pixelate, ...) capabilities. ‌I use flameshot not because it's the only software that takes photos of my screen. I use it because it's feature rich and I really miss the OCR feature.

@mmahmoudian
Copy link
Member

mmahmoudian commented Apr 28, 2024

@erfanium

I think this argument can also be applied to the currently implemented post-processing (draw shape, pen, pixelate, ...)

Flameshot is a screenshot annotation tool. All those features you mentioned are necessary for annotating screenshots. OCR has nothing to do with annotation.

I use it because it's feature rich and I really miss the OCR feature.

Then please go and support the plugin system with which this feature will be available.

This was a unanimous decision by all devs to only allow such features as plugin. One of the reason for this is extremely high maintenance burden of such features on all supported operating systems.


@debuglevel

I'm always starting https://github.com/ShareX/ShareX to do a quick OCR. It just works.

They only support Windows. We support Linux, Mac, Windows and even BSD.


@rsrdesarrollo

Let me, greenshot, sharex and snagit disagree, OCR is indeed a great and wanted feature for screenshot tools.

For now, the policy of maintainers is to keep it screenshot and annotation specific. Any other gizmo (watermark, printing, GIF, OCR, barcode reader,...) should only and only be allowed through a plugin system. This is to help us keep this project maintainable and prevent it from bloat.

But that said, Flameshot is a FLOSS tool (emphasis on "free" and "libre"), therefore, anyone is legally allowed to maintain their own fork based on the restrictions imposed by the license of Flameshot.

Also they are all Windows-only tools and are not cross-platform, or commercial in case od Snagit.

In my humble opinion, here is a PR to allow 90% of your users to do what they need today

You are right, it will allow some some users do OCR today, but maintaining it will be an everlasting burden on devs. You kindly provided a PR but you are not taking the responsibility to take care of this part of the code for ever. Do you?

I also don't think 90% of Flameshot users are in need of OCR. We don't have telemetry, so we don't have statistics on our user count, but I bet the number you should aim for is less than 1%.

AFAIK plugins system is only a concept, nothing implemented.

True, it is not implemented yet (at least not yet landed on the main branch). But:

  1. If there is enough demand, someone (our dev or someone from community) would start working on it
  2. We are in negotiation with a company. They need certain features that we would like to provide only as plugin. They agreed to commission this. If things goes well and if we find a dev who would like to do this big task, then we will have the plugin system.

I thank you again for your PR. But I think you can also agree that in general:

  1. Policy is policy, especially when it is democratically decided upon
  2. Attacking/pressuring devs of a FLOSS project who are all volunteers is not constructive (emphasis on "in general")

@debuglevel
Copy link

debuglevel commented Apr 28, 2024

@debuglevel

I'm always starting https://github.com/ShareX/ShareX to do a quick OCR. It just works.

They only support Windows. We support Linux, Mac, Windows and even BSD.

Yes...? So does tesseract.

Please just close this issue instead of mentioning "create your own fork", "submit a PR" or other FOSS clichés to end-users ;-).

@GrabbenD
Copy link

GrabbenD commented Apr 28, 2024

I agree with @mmahmoudian

To keep bloat to minimum, ensure low maintenance burden and provide high cross-platform compatibility, it just doesn't make sense to have advanced tools like OCR as part of the core project. Furthermore, alternative utilities such as ShareX are clogged with unnecessary tools.. I hope Flameshot never gets to that state.

Plugin system is a very reasonable solution. From my experience, even a highly maintained project like Tesseract is prone to providing bad results without tuning. Having plug-and-play plugins would also allow for easier ability to tweak using platform specific tools rather than having to re-compile the entire project.

This really sums up the reason why I enjoyed this PR in the first place:

It checks for Tesseract in PATH meaning the feature is opt-in and uses upstream updates rather than a statically linked library (which would inevitably become outdated)

@mmahmoudian
Copy link
Member

@GrabbenD

This really sums up the reason why I enjoyed this PR in the first place

The only reason I've kept this PR open is due it's simplicity and straightforward implementation. I am hoping that we can somehow incorporate this PR as an add-on so that @rsrdesarrollo can maintain this add-on instead a a full fork. At the end of the day, I'm one person in a team and I should not and cannot act alone.


@debuglevel This is a fully volunteer-based FLOSS project. We are not getting paid to serve you, nor we owe you anything! This is our project and we maintain and manage it as we see fit. Lastly, be civil; we have zero tolerance policy for trolls.

@debuglevel
Copy link

Yeah, therefore just close the issue so everybody knows this is a "won't fix". Keeping it open does not help anybody but just attracts more people who hope this will get implemented.

@GrabbenD
Copy link

@debuglevel

I am hoping that we can somehow incorporate this PR as an add-on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OCR to clipboard hook for selections
5 participants