Hi there, Attaching the patch that we've applied on Debian to fix the following:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=745888 Thanks for considering, and cheers! -- Alessio Treglia | www.alessiotreglia.com Debian Developer | [email protected] Ubuntu Core Developer | [email protected] 0416 0004 A827 6E40 BB98 90FB E8A4 8AE5 311D 765A
From 07f467ad5d151294ad6a9607ed75e4bbcc7c5fd4 Mon Sep 17 00:00:00 2001 From: David Bremner <[email protected]> Date: Fri, 13 Jun 2014 17:48:56 -0300 Subject: [PATCH] Revert "Async buffering for high-speed transfers." This reverts commit b9a840cc79bb96f135fe4abd4248881240026112. Conflicts: src/libusb-glue.h The only non-obvious conflict is because of later changes to how libusb.h is included --- src/libusb-glue.h | 32 ---- src/libusb1-glue.c | 519 ++--------------------------------------------------- 2 files changed, 11 insertions(+), 540 deletions(-) diff --git a/src/libusb-glue.h b/src/libusb-glue.h index 5ba6183..0ea6b29 100644 --- a/src/libusb-glue.h +++ b/src/libusb-glue.h @@ -33,7 +33,6 @@ #include "ptp.h" #ifdef HAVE_LIBUSB1 #include <libusb.h> -#include <pthread.h> #endif #ifdef HAVE_LIBUSB0 #include <usb.h> @@ -77,20 +76,6 @@ extern "C" { #define USB_BULK_WRITE openusb_bulk_xfer #endif -#ifdef HAVE_LIBUSB1 -/* - * With libusb1 we use a threaded read/write handler - * to speed up transfers. - */ -struct ptp_write_td { - struct libusb_transfer *transfer; - struct ptp_write_td *next; - unsigned char *buffer; -}; -/* Number of async transfer buffers */ -#define WRITE_TRANSFER_NUM 4 -#endif - /** * Internal USB struct. */ @@ -98,24 +83,7 @@ typedef struct _PTP_USB PTP_USB; struct _PTP_USB { PTPParams *params; #ifdef HAVE_LIBUSB1 - /* - * With libusb1 we use a threaded read/write handler - * to speed up transfers. - */ libusb_device_handle* handle; - int event_thread_run; - pthread_t ptp_event_thread_tid; - int ptp_read_transfer_done_flag; - pthread_cond_t ptp_read_transfer_done_cv; - pthread_mutex_t ptp_read_transfer_done_mutex; - - int ptp_write_transfer_done_flag; - pthread_cond_t ptp_write_transfer_cv; - pthread_mutex_t ptp_write_transfer_mutex; - - unsigned char *bytes[WRITE_TRANSFER_NUM]; - struct ptp_write_td write_td_array[WRITE_TRANSFER_NUM]; - struct ptp_write_td *p_write_head_td, *p_write_tail_td; #endif #ifdef HAVE_LIBUSB0 usb_dev_handle* handle; diff --git a/src/libusb1-glue.c b/src/libusb1-glue.c index 53511ab..333a34a 100644 --- a/src/libusb1-glue.c +++ b/src/libusb1-glue.c @@ -29,19 +29,19 @@ * use big-endian dates.) * */ -#include <errno.h> -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#include <unistd.h> -#include <signal.h> - #include "config.h" #include "libmtp.h" #include "libusb-glue.h" #include "device-flags.h" #include "util.h" #include "ptp.h" + +#include <errno.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + #include "ptp-pack.c" /* @@ -378,7 +378,7 @@ static int probe_device_descriptor(libusb_device *dev, FILE *dumpfile) */ if (ret < 0) { /* EP0 is the default control endpoint */ - libusb_clear_halt(devh, 0); + libusb_clear_halt (devh, 0); libusb_close(devh); return 0; } @@ -804,265 +804,6 @@ libusb_glue_error (PTPParams *params, const char *format, ...) } -static void *ptp_event_thread_func(void *arg) -{ - PTP_USB *ptp_usb = (PTP_USB*)arg; - - LIBMTP_USB_DEBUG("event thread start\n"); - ptp_usb->event_thread_run = 1; - - while (ptp_usb->event_thread_run) - libusb_handle_events(NULL); - - LIBMTP_USB_DEBUG("event thread quit\n"); - return NULL; -} - -static int ptp_write_async_transfer_init(PTP_USB *ptp_usb); -static void ptp_write_async_transfer_cleanup(PTP_USB *ptp_usb); - -static int ptp_event_thread_init(PTP_USB *ptp_usb) -{ - int retval; - - LIBMTP_USB_DEBUG("ptp event thread init\n"); - retval = pthread_mutex_init(&(ptp_usb->ptp_read_transfer_done_mutex), NULL); - if (retval) { - perror("create mutex error\n"); - return 1; - } - retval = pthread_cond_init(&(ptp_usb->ptp_read_transfer_done_cv), NULL); - if (retval) { - perror("create codintion variable error\n"); - return 1; - } - - ptp_usb->ptp_read_transfer_done_flag = 0; - - retval = pthread_mutex_init(&(ptp_usb->ptp_write_transfer_mutex), NULL); - if (retval) { - perror("create mutex error\n"); - return 1; - } - retval = pthread_cond_init(&(ptp_usb->ptp_write_transfer_cv), NULL); - if (retval) { - perror("create codintion variable error\n"); - return 1; - } - - ptp_usb->ptp_write_transfer_done_flag = 0; - ptp_usb->event_thread_run = 0; - ptp_usb->ptp_event_thread_tid = 0; - - if (!ptp_usb->ptp_event_thread_tid) { - if (pthread_create(&(ptp_usb->ptp_event_thread_tid), NULL, ptp_event_thread_func, ptp_usb) != 0) { - perror("create event thread fail\n"); - return -1; - } - } - - retval = ptp_write_async_transfer_init(ptp_usb); - if (retval) - perror("init write async error"); - return 0; - -} - -static void ptp_event_thread_cleanup(PTP_USB *ptp_usb) -{ - ptp_write_async_transfer_cleanup(ptp_usb); - pthread_mutex_destroy(&(ptp_usb->ptp_read_transfer_done_mutex)); - pthread_cond_destroy(&(ptp_usb->ptp_read_transfer_done_cv)); - pthread_mutex_destroy(&(ptp_usb->ptp_write_transfer_mutex)); - pthread_cond_destroy(&(ptp_usb->ptp_write_transfer_cv)); - pthread_join(ptp_usb->ptp_event_thread_tid, NULL); - ptp_usb->ptp_event_thread_tid = 0; - LIBMTP_USB_DEBUG("finish cleanup event thread\n"); -} - -static void ptp_read_transfer_cb(struct libusb_transfer *transfer) -{ - PTP_USB *ptp_usb = (PTP_USB*)transfer->user_data; - - pthread_mutex_lock(&(ptp_usb->ptp_read_transfer_done_mutex)); - ptp_usb->ptp_read_transfer_done_flag = 1; - pthread_cond_signal(&(ptp_usb->ptp_read_transfer_done_cv)); - pthread_mutex_unlock(&(ptp_usb->ptp_read_transfer_done_mutex)); -} - -#define READ_ASYNC_BLOCK_SIZE (0x4000*8) - -static short -ptp_read_async_func ( - unsigned long size, PTPDataHandler *handler,void *data, - unsigned long *readbytes, - int readzero -) { - PTP_USB *ptp_usb = (PTP_USB *)data; - unsigned long toread = 0; - int ret = 0; - int xread = 0; - unsigned long curread = 0; - unsigned long written; - unsigned char *bytes[2]; - unsigned int transfer_index = 0; - int expect_terminator_byte = 0; - struct libusb_transfer *usb_read_transfer[2]; - struct libusb_transfer *usb_ongoing_transfer = NULL; - struct libusb_transfer *usb_done_transfer = NULL; - unsigned long sync_written = 0; - - // This is the largest block we'll need to read in. - bytes[0] = malloc(READ_ASYNC_BLOCK_SIZE); - if (!bytes[0]) - return PTP_ERROR_IO; - bytes[1] = malloc(READ_ASYNC_BLOCK_SIZE); - if (!bytes[1]) { - if (bytes[0]) - free(bytes[0]); - return PTP_ERROR_IO; - } - - usb_read_transfer[0] = libusb_alloc_transfer(0); - if (!usb_read_transfer[0]) - return PTP_ERROR_IO; - usb_read_transfer[1] = libusb_alloc_transfer(0); - if (!usb_read_transfer[1]) { - if (usb_read_transfer[0]) - libusb_free_transfer(usb_read_transfer[0]); - return PTP_ERROR_IO; - } - while (curread < size || usb_done_transfer) { - - LIBMTP_USB_DEBUG("Remaining size to read: 0x%04lx bytes\n", size - curread); - - // check equal to condition here - if (size - curread < READ_ASYNC_BLOCK_SIZE) - { - // this is the last packet - toread = size - curread; - // this is equivalent to zero read for these devices - if (readzero && FLAG_NO_ZERO_READS(ptp_usb) && toread % 64 == 0) { - toread += 1; - expect_terminator_byte = 1; - } - } - else - toread = READ_ASYNC_BLOCK_SIZE; - - - LIBMTP_USB_DEBUG("Reading in 0x%04lx bytes\n", toread); - - if (toread > 0) { - transfer_index = (transfer_index + 1) % 2; - libusb_fill_bulk_transfer(usb_read_transfer[transfer_index], - ptp_usb->handle, - ptp_usb->inep, - bytes[transfer_index], - toread, - ptp_read_transfer_cb, - ptp_usb, - 2000); - ptp_usb->ptp_read_transfer_done_flag = 0; - ret = libusb_submit_transfer(usb_read_transfer[transfer_index]); - if (ret) - perror("submit error:"); - usb_ongoing_transfer = usb_read_transfer[transfer_index]; - } - - if (usb_done_transfer) { - int putfunc_ret = handler->putfunc(NULL, handler->priv, xread, usb_done_transfer->buffer, &written); - sync_written += written; - if (putfunc_ret != PTP_RC_OK) - return putfunc_ret; - usb_done_transfer = NULL; - } - - if (usb_ongoing_transfer) { - pthread_mutex_lock(&(ptp_usb->ptp_read_transfer_done_mutex)); - while (!ptp_usb->ptp_read_transfer_done_flag) { - pthread_cond_wait(&(ptp_usb->ptp_read_transfer_done_cv), &(ptp_usb->ptp_read_transfer_done_mutex)); - } - pthread_mutex_unlock(&(ptp_usb->ptp_read_transfer_done_mutex)); - - if (usb_ongoing_transfer->status == LIBUSB_TRANSFER_COMPLETED) { - xread = usb_ongoing_transfer->actual_length; - LIBMTP_USB_DEBUG("Result of read: 0x%04x (%d bytes)\n", ret, xread); - - LIBMTP_USB_DEBUG("<==USB IN\n"); - if (xread == 0) - LIBMTP_USB_DEBUG("Zero Read\n"); - else - LIBMTP_USB_DATA(usb_ongoing_transfer->buffer, xread, 16); - - // want to discard extra byte - if (expect_terminator_byte && xread == toread) - { - LIBMTP_USB_DEBUG("<==USB IN\nDiscarding extra byte\n"); - xread--; - } - ptp_usb->current_transfer_complete += xread; - curread += xread; - usb_done_transfer = usb_ongoing_transfer; - usb_ongoing_transfer = NULL; - } else { - perror("transfer error:"); - return PTP_ERROR_IO; - } - } - - // Increase counters, call callback - if (ptp_usb->callback_active) { - if (ptp_usb->current_transfer_complete >= ptp_usb->current_transfer_total) { - // send last update and disable callback. - ptp_usb->current_transfer_complete = ptp_usb->current_transfer_total; - ptp_usb->callback_active = 0; - } - if (ptp_usb->current_transfer_callback != NULL) { - int ret; - ret = ptp_usb->current_transfer_callback(ptp_usb->current_transfer_complete, - ptp_usb->current_transfer_total, - ptp_usb->current_transfer_callback_data); - if (ret != 0) { - return PTP_ERROR_CANCEL; - } - } - } - - if (xread < toread) /* short reads are common */ - break; - } - - if (readbytes) *readbytes = curread; - free (bytes[0]); - free (bytes[1]); - libusb_free_transfer(usb_read_transfer[0]); - libusb_free_transfer(usb_read_transfer[1]); - - // there might be a zero packet waiting for us... - if (readzero && - !FLAG_NO_ZERO_READS(ptp_usb) && - curread % ptp_usb->outep_maxpacket == 0) { - unsigned char temp; - int zeroresult = 0, xread; - - //printf("zero read\n"); - LIBMTP_USB_DEBUG("<==USB IN\n"); - LIBMTP_USB_DEBUG("Zero Read\n"); - - zeroresult = USB_BULK_READ(ptp_usb->handle, - ptp_usb->inep, - &temp, - 0, - &xread, - ptp_usb->timeout); - if (zeroresult != LIBUSB_SUCCESS) - LIBMTP_INFO("LIBMTP panic: unable to read in zero packet, response 0x%04x", zeroresult); - } - - return PTP_RC_OK; -} - /* * ptp_read_func() and ptp_write_func() are * based on same functions usb.c in libgphoto2. @@ -1233,241 +974,6 @@ ptp_read_func ( return PTP_RC_OK; } -#define WRITE_ASYNC_BLOCK_SIZE (0x4000) - - -static struct ptp_write_td *ptp_get_write_transfer(PTP_USB *ptp_usb) -{ - struct ptp_write_td *write_td; - - pthread_mutex_lock(&(ptp_usb->ptp_write_transfer_mutex)); - if (ptp_usb->p_write_head_td) { - if (ptp_usb->p_write_head_td == ptp_usb->p_write_tail_td) { - /* hit the last one */ - ptp_usb->p_write_tail_td = NULL; - ptp_usb->ptp_write_transfer_done_flag = 0; - } - write_td = ptp_usb->p_write_head_td; - ptp_usb->p_write_head_td = ptp_usb->p_write_head_td->next; - write_td->next = NULL; - } else { - /* empty list */ - while (!(ptp_usb->ptp_write_transfer_done_flag)) { - pthread_cond_wait(&(ptp_usb->ptp_write_transfer_cv), &(ptp_usb->ptp_write_transfer_mutex)); - } - write_td = ptp_usb->p_write_head_td; - ptp_usb->p_write_head_td = ptp_usb->p_write_head_td->next; - - if (ptp_usb->p_write_head_td == NULL) { - ptp_usb->p_write_tail_td = NULL; - ptp_usb->ptp_write_transfer_done_flag = 0; - } - write_td->next = NULL; - } - pthread_mutex_unlock(&(ptp_usb->ptp_write_transfer_mutex)); - - return write_td; -} - -static void ptp_giveback_write_transfer(PTP_USB *ptp_usb, struct ptp_write_td *ptp_write_transfer) -{ - pthread_mutex_lock(&(ptp_usb->ptp_write_transfer_mutex)); - if (ptp_usb->p_write_tail_td) { - ptp_usb->p_write_tail_td->next = ptp_write_transfer; - ptp_usb->p_write_tail_td = ptp_write_transfer; - ptp_write_transfer->next = NULL; - } else { - /* empty list */ - ptp_usb->p_write_tail_td = ptp_write_transfer; - ptp_usb->p_write_head_td = ptp_usb->p_write_tail_td; - ptp_write_transfer->next = NULL; - ptp_usb->ptp_write_transfer_done_flag = 1; - pthread_cond_signal(&(ptp_usb->ptp_write_transfer_cv)); - } - - pthread_mutex_unlock(&(ptp_usb->ptp_write_transfer_mutex)); -} - -static struct ptp_write_td *libusb_transfer_to_td(PTP_USB *ptp_usb, struct libusb_transfer *transfer) -{ - int i = 0; - - for (i = 0; i < WRITE_TRANSFER_NUM; i++) { - if (ptp_usb->write_td_array[i].transfer == transfer) { - return &(ptp_usb->write_td_array[i]); - } - } - perror("no corresponding td found"); - return NULL; -} - -static void ptp_write_transfer_cb(struct libusb_transfer *transfer) -{ - PTP_USB *ptp_usb = (PTP_USB*)transfer->user_data; - struct ptp_write_td *giveback_td; - - giveback_td = libusb_transfer_to_td(ptp_usb, transfer); - ptp_giveback_write_transfer(ptp_usb, giveback_td); -} - -static int ptp_write_async_transfer_init(PTP_USB *ptp_usb) -{ - int i; - - for (i = 0; i < WRITE_TRANSFER_NUM; i++) { - ptp_usb->bytes[i] = malloc(WRITE_ASYNC_BLOCK_SIZE); - if (!ptp_usb->bytes[i]) { - perror("alloc write buffer error"); - goto init_err; - } - } - - for (i = 0; i < WRITE_TRANSFER_NUM; i++) { - ptp_usb->write_td_array[i].transfer = libusb_alloc_transfer(0); - if (!ptp_usb->write_td_array[i].transfer) { - perror("alloc write transfer error"); - goto init_err; - } - ptp_usb->write_td_array[i].buffer = ptp_usb->bytes[i]; - } - return 0; - init_err: - for (i = 0; i < WRITE_TRANSFER_NUM; i++) { - if (ptp_usb->bytes[i]) { - free(ptp_usb->bytes[i]); - ptp_usb->bytes[i] = NULL; - } - if (ptp_usb->write_td_array[i].transfer) { - libusb_free_transfer(ptp_usb->write_td_array[i].transfer); - ptp_usb->write_td_array[i].transfer = NULL; - } - } - return -1; -} - -static void ptp_write_async_transfer_cleanup(PTP_USB *ptp_usb) -{ - int i; - - for (i = 0; i < WRITE_TRANSFER_NUM; i++) { - if (ptp_usb->write_td_array[i].transfer) { - libusb_free_transfer(ptp_usb->write_td_array[i].transfer); - ptp_usb->write_td_array[i].transfer = NULL; - } - if (ptp_usb->write_td_array[i].buffer) { - free(ptp_usb->write_td_array[i].buffer); - ptp_usb->write_td_array[i].buffer = NULL; - } - } -} - -static short -ptp_write_async_func ( - unsigned long size, - PTPDataHandler *handler, - void *data, - unsigned long *written -) { - PTP_USB *ptp_usb = (PTP_USB *)data; - unsigned long towrite = 0; - int ret = 0; - unsigned long curwrite = 0; - struct ptp_write_td *write_td = NULL; - int i; - - for (i = 0; i < WRITE_TRANSFER_NUM; i++) { - if (i < WRITE_TRANSFER_NUM - 1) - ptp_usb->write_td_array[i].next = &(ptp_usb->write_td_array[i + 1]); - else - ptp_usb->write_td_array[i].next = NULL; - } - ptp_usb->p_write_head_td = &(ptp_usb->write_td_array[0]); - ptp_usb->p_write_tail_td = &(ptp_usb->write_td_array[WRITE_TRANSFER_NUM - 1]); - - while (curwrite < size) { - towrite = size-curwrite; - if (towrite > WRITE_ASYNC_BLOCK_SIZE) { - towrite = WRITE_ASYNC_BLOCK_SIZE; - } else { - // This magic makes packets the same size that WMP send them. - if (towrite > ptp_usb->outep_maxpacket && towrite % ptp_usb->outep_maxpacket != 0) { - towrite -= towrite % ptp_usb->outep_maxpacket; - } - } - write_td = ptp_get_write_transfer(ptp_usb); - if (!write_td) { - perror("get NULL write transfer"); - return PTP_ERROR_IO; - } - int getfunc_ret = handler->getfunc(NULL, handler->priv,towrite,write_td->buffer,&towrite); - if (getfunc_ret != PTP_RC_OK) { - return getfunc_ret; - } - - libusb_fill_bulk_transfer(write_td->transfer, - ptp_usb->handle, - ptp_usb->outep, - write_td->buffer, - towrite, - ptp_write_transfer_cb, - ptp_usb, - 2000); - ret = libusb_submit_transfer(write_td->transfer); - if (ret) - perror("submit write transfer error:"); - - - LIBMTP_USB_DEBUG("USB OUT==>\n"); - - // Increase counters - ptp_usb->current_transfer_complete += towrite; - curwrite += towrite; - // call callback - if (ptp_usb->callback_active) { - if (ptp_usb->current_transfer_complete >= ptp_usb->current_transfer_total) { - // send last update and disable callback. - ptp_usb->current_transfer_complete = ptp_usb->current_transfer_total; - ptp_usb->callback_active = 0; - } - if (ptp_usb->current_transfer_callback != NULL) { - int ret; - ret = ptp_usb->current_transfer_callback(ptp_usb->current_transfer_complete, - ptp_usb->current_transfer_total, - ptp_usb->current_transfer_callback_data); - if (ret != 0) { - return PTP_ERROR_CANCEL; - } - } - } - - } - - if (written) { - *written = curwrite; - } - - // If this is the last transfer send a zero write if required - if (ptp_usb->current_transfer_complete >= ptp_usb->current_transfer_total) { - if ((towrite % ptp_usb->outep_maxpacket) == 0) { - int xwritten; - - LIBMTP_USB_DEBUG("USB OUT==>\n"); - LIBMTP_USB_DEBUG("Zero Write\n"); - ret =USB_BULK_WRITE(ptp_usb->handle, - ptp_usb->outep, - (unsigned char *) "x", - 0, - &xwritten, - ptp_usb->timeout); - } - } - - if (ret != LIBUSB_SUCCESS) - return PTP_ERROR_IO; - - return PTP_RC_OK; -} - static short ptp_write_func ( unsigned long size, @@ -1767,7 +1273,7 @@ ptp_usb_senddata (PTPParams* params, PTPContainer* ptp, bytes_left_to_transfer = size-datawlen; ret = PTP_RC_OK; while(bytes_left_to_transfer > 0) { - ret = ptp_write_async_func (bytes_left_to_transfer, handler, params->data, &written); + ret = ptp_write_func (bytes_left_to_transfer, handler, params->data, &written); if (ret != PTP_RC_OK) break; if (written == 0) { @@ -1875,7 +1381,7 @@ ptp_usb_getdata (PTPParams* params, PTPContainer* ptp, PTPDataHandler *handler) unsigned long readdata; uint16_t xret; - xret = ptp_read_async_func( + xret = ptp_read_func( 0x20000000, handler, params->data, @@ -1980,7 +1486,7 @@ ptp_usb_getdata (PTPParams* params, PTPContainer* ptp, PTPDataHandler *handler) break; } - ret = ptp_read_async_func(len - (rlen - PTP_USB_BULK_HDR_LEN), + ret = ptp_read_func(len - (rlen - PTP_USB_BULK_HDR_LEN), handler, params->data, &rlen, 1); @@ -2378,9 +1884,7 @@ static void close_usb(PTP_USB* ptp_usb) */ libusb_reset_device (ptp_usb->handle); } - ptp_usb->event_thread_run = 0; libusb_close(ptp_usb->handle); - ptp_event_thread_cleanup(ptp_usb); } /** @@ -2611,7 +2115,6 @@ LIBMTP_error_number_t configure_usb_device(LIBMTP_raw_device_t *device, /* OK configured properly */ *usbinfo = (void *) ptp_usb; libusb_free_device_list (devs, 0); - ptp_event_thread_init(ptp_usb); return LIBMTP_ERROR_NONE; } -- 2.0.0
