Comment 31 for bug 790183

Revision history for this message
In , anarsoul (anarsoul) wrote :

Comment on attachment 75783
Patch (against latest git version) with driver implementation

Review of attachment 75783:
-----------------------------------------------------------------

::: libfprint/drivers/vfs5011.c
@@ +25,5 @@
> +}
> +
> +static void debugprint(const char *line)
> +{
> +#ifdef DEBUG

There's fp_dbg/fp_err/fp_whatever already. Please don't reinvent a wheel

@@ +67,5 @@
> +static int usb_send(libusb_device_handle *handle, unsigned char *data, unsigned size)
> +{
> + int transferred = 0;
> + int r = libusb_bulk_transfer(handle, VFS5011_OUT_ENDPOINT, data, size,
> + &transferred, VFS5011_DEFAULT_WAIT_TIMEOUT);

No synchronous transfers are allowed in driver

@@ +85,5 @@
> +static int usb_recv(libusb_device_handle *handle, unsigned endpoint, unsigned char *buf,
> + unsigned max_bytes, int *transferred)
> +{
> + int r = libusb_bulk_transfer(handle, endpoint, buf, max_bytes, transferred,
> + VFS5011_DEFAULT_WAIT_TIMEOUT);

Same

@@ +404,5 @@
> +
> +// Rescale image to account for variable swiping speed
> +int vfs5011_rescale_image(unsigned char *image, int input_lines,
> + unsigned char *output, int max_output_lines)
> +{

Could you use existing scale functions?

@@ +435,5 @@
> +// if (! on_good_offsets && (i >= GOOD_OFFSETS_CRITERION)) {
> +// if (get_deviation_int(offsets + i - GOOD_OFFSETS_CRITERION, GOOD_OFFSETS_CRITERION) <
> +// GOOD_OFFSETS_THRESHOLD)
> +// on_good_offsets = 1;
> +// }

Please drop commented out blocks of code

@@ +876,5 @@
> +struct fp_img_driver vfs5011_driver =
> +{
> + .driver =
> + {
> + .id = 12,

Please don't use hardcoded ID anymore, define your own in driver_ids.h and use that one

@@ +886,5 @@
> +
> + .flags = 0,
> + .img_width = VFS5011_IMAGE_WIDTH,
> + .img_height = -1,
> + .bz3_threshold = 20,

Why threshold is so low?