Add location/position support #12
Conversation
There was a problem hiding this comment.
Looks really good - thanks again for submitting this PR. I have made few suggestions for how to return a bounding box around barcodes. We could in future support returning the complete list of locations and/or computing a polygon around barcodes.
If you agree with these changes, the README and unit tests will need to be altered.
pyzbar/wrapper.py
Outdated
|
|
||
| zbar_symbol_get_loc_x = zbar_function( | ||
| 'zbar_symbol_get_loc_x', | ||
| c_uint, |
There was a problem hiding this comment.
My copy of zbar.h has the return type as int, which is represented by c_int. Could you check the type in your copy of the header?
pyzbar/wrapper.py
Outdated
|
|
||
| zbar_symbol_get_loc_y = zbar_function( | ||
| 'zbar_symbol_get_loc_y', | ||
| c_uint, |
There was a problem hiding this comment.
My copy of zbar.h has the return type as int, which is represented by c_int. Could you check the type in your copy of the header?
pyzbar/pyzbar.py
Outdated
| @@ -11,14 +11,15 @@ | |||
| zbar_image_create, zbar_image_destroy, zbar_image_set_format, | |||
| zbar_image_set_size, zbar_image_set_data, zbar_scan_image, | |||
| zbar_image_first_symbol, zbar_symbol_get_data, | |||
| zbar_symbol_get_loc_size, zbar_symbol_get_loc_x, zbar_symbol_get_loc_y, | |||
| zbar_symbol_next, ZBarConfig, ZBarSymbol, EXTERNAL_DEPENDENCIES | |||
| ) | |||
|
|
|||
| __all__ = ['decode', 'EXTERNAL_DEPENDENCIES'] | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Suggest that we return a single rectangle that describes the bounding box around the detected barcode:
# A rectangle
Rect = namedtuple('Rect', ['left', 'top', 'width', 'height'])
# Results of reading a barcode
Decoded = namedtuple('Decoded', ['data', 'type', 'rect'])
pyzbar/pyzbar.py
Outdated
| zbar_symbol_get_loc_x(symbol, l), | ||
| zbar_symbol_get_loc_y(symbol, l) | ||
| ) | ||
| for l in range(loc) |
There was a problem hiding this comment.
Prefer xrange on Python 2.x . At the top of the module (or maybe within this function):
RANGEFN = getattr(globals(), 'xrange', range)
Alter range(loc) to RANGEFN(loc)
pyzbar/pyzbar.py
Outdated
| results.append(Decoded( | ||
| data=data, | ||
| type=symbol_type | ||
| type=symbol_type, | ||
| location=location |
There was a problem hiding this comment.
Replace with rect=bounding_box_of_locations(locations)
pyzbar/pyzbar.py
Outdated
| zbar_symbol_next, ZBarConfig, ZBarSymbol, EXTERNAL_DEPENDENCIES | ||
| ) | ||
|
|
||
| __all__ = ['decode', 'EXTERNAL_DEPENDENCIES'] | ||
|
|
||
|
|
||
| # Results of reading a barcode | ||
| Decoded = namedtuple('Decoded', ['data', 'type']) | ||
| Decoded = namedtuple('Decoded', ['data', 'type', 'location']) | ||
|
|
There was a problem hiding this comment.
A function to compute bounding boxes from scan locations (uses operator.itemgetter):
def bounding_box_of_locations(locations):
x_values = list(map(itemgetter(0), locations))
x_min, x_max = min(x_values), max(x_values)
y_values = list(map(itemgetter(1), locations))
y_min, y_max = min(y_values), max(y_values)
return (Rect(x_min, y_min, x_max - x_min, y_max - y_min))
pyzbar/pyzbar.py
Outdated
|
|
||
| location = [] | ||
| loc = zbar_symbol_get_loc_size(symbol) | ||
| if loc: |
There was a problem hiding this comment.
I am curious - does zbar_symbol_get_loc_size ever return 0? I have done some limited testing on Mac OS X and have always seen a non-zero return.
Return a bounding box around barcodes. Corrected mistakes with ctypes. Eliminated superfluous check of the result of zbar_symbol_get_loc_size function call, as well as the mess I introduced into tests.
|
Updated the pull request. Thank you for your suggestions. |
|
Fantastic! Thanks for the very fast update @globophobe |
Should fix issue #11. Tests pass.
It's my first time using ctypes. Hopefully, there aren't any mistakes.