Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

feat: quicker scans by faster number aggregation #32

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

Conversation

m-schmoock
Copy link
Contributor

@m-schmoock m-schmoock commented May 12, 2016

I think we can get rid of the minimal 3 frames requirement by simplifying the 15/16 number score aggregation:

  • always apply exponential backoff to all aggregates
  • increase just the matching one
  • take the bigger aggregated.sum() for decision instead of waiting at least three frames
  • additionally we can remove the 15/16 counters.

I tested it with all my cards. It did not increase false positives, but it did decreases scan time quite noticeable. The code is simpler and I think more effective.

Please give it a try.

@braebot
Copy link
Member

braebot commented May 23, 2016

Ping @dgoldman-ebay and @josharian. Any assistance here would be greatly appreciated. :)

@dgoldman-pdx
Copy link
Member

I'm not comfortable advising you guys on this one; I'd have to delve back into the code more than I currently have time for. Also, this is an area the @josharian worked on a lot, back in the day. Probably he Knows Stuff that would be helpful.

@josharian
Copy link
Member

The right way to make decisions like this is with lots of data. I no longer have access to my test data sets, so I can't say anything definitely. And as part of open-sourcing we removed our analytics code, so we can't really gather data from the field.

@m-schmook (or anyone else!), if you have a big install base and want to do some instrumentation, try an a/b test, and come back with some data, it'd make it much easier to go for this.

I do recall that there are definitely cards for which we get occasional bad 15 vs 16 decisions from vseg. Just taking the first thing that comes across the board will increase false positives. The user experience for false positives is much worse than for slower scans, so we generally skew towards slow.

@m-schmoock
Copy link
Contributor Author

Okay, I will do some more testing and documentation.
Therefore, can you give me some of your test data?
I think my set is too limited.

Also, I'm thinking to change if (state->aggregated15.sum() > state->aggregated16.sum()) { to something more suitable.

@tomwhipple
Copy link
Member

Sadly the test data in question is credit card images, so they can't really be released. :(

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

Successfully merging this pull request may close these issues.

5 participants