Skip to content

Commit

Permalink
Behave sanely when the Kinect is unplugged.
Browse files Browse the repository at this point in the history
libusb_handle_events is thread-safe, but not reentrant.  Which means that we
can't call fnusb_stop_iso which calls libusb_handle_events from inside the
isochronous callback function.

Instead, we add a member to fnusb_dev that tracks if the device is permanently
dead, and set it to true when we get a LIBUSB_TRANSFER_NO_DEVICE (or on resubmission,
LIBUSB_ERROR_NO_DEVICE).

Then, in freenect_process_events, after calling libusb_handle_events
internally, we check to see if any of the devices in the context have this flag
set.  If they do, then we should return some nonzero error value, which is
currently -1.

In the future, we intend to provide callbacks so that clients can receive
explicit notifications when a device has disappeared, so the code can robustly
handle that situation.  Future work.

Fixes #229 (I think)

Signed-off-by: Drew Fisher <drew.m.fisher@gmail.com>
  • Loading branch information
zarvox committed Nov 21, 2011
1 parent 75a3eb2 commit 3f7e959
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 9 deletions.
26 changes: 24 additions & 2 deletions src/core.c
Expand Up @@ -69,12 +69,34 @@ FREENECTAPI int freenect_shutdown(freenect_context *ctx)

FREENECTAPI int freenect_process_events(freenect_context *ctx)
{
return fnusb_process_events(&ctx->usb);
struct timeval timeout;
timeout.tv_sec = 60;
timeout.tv_usec = 0;
return freenect_process_events_timeout(ctx, &timeout);
}

FREENECTAPI int freenect_process_events_timeout(freenect_context *ctx, struct timeval *timeout)
{
return fnusb_process_events_timeout(&ctx->usb, timeout);
int res = fnusb_process_events_timeout(&ctx->usb, timeout);
// Iterate over the devices in ctx. If any of them are flagged as
freenect_device* dev = ctx->first;
while(dev) {
if (dev->usb_cam.device_dead) {
FN_ERROR("USB camera marked dead, stopping streams\n");
res = -1;
freenect_stop_video(dev);
freenect_stop_depth(dev);
}
#ifdef BUILD_AUDIO
if (dev->usb_audio.device_dead) {
FN_ERROR("USB audio marked dead, stopping streams\n");
res = -1; // Or something else to tell the user that the device just vanished.
freenect_stop_audio(dev);
}
#endif
dev = dev->next;
}
return res;
}

FREENECTAPI int freenect_num_devices(freenect_context *ctx)
Expand Down
35 changes: 28 additions & 7 deletions src/usb_libusb10.c
Expand Up @@ -442,8 +442,8 @@ static void iso_callback(struct libusb_transfer *xfer)
if (res != 0) {
FN_ERROR("iso_callback(): failed to resubmit transfer after successful completion: %d\n", res);
strm->dead_xfers++;
if (res == LIBUSB_TRANSFER_NO_DEVICE) {
fnusb_stop_iso(strm->parent, strm);
if (res == LIBUSB_ERROR_NO_DEVICE) {
strm->parent->device_dead = 1;
}
}
break;
Expand All @@ -453,14 +453,28 @@ static void iso_callback(struct libusb_transfer *xfer)
// We lost the device we were talking to. This is a large problem,
// and one that we should eventually come up with a way to
// properly propagate up to the caller.
FN_ERROR("USB device disappeared, cancelling stream :(\n");
if(!strm->parent->device_dead) {
FN_ERROR("USB device disappeared, cancelling stream %02x :(\n", xfer->endpoint);
}
strm->dead_xfers++;
fnusb_stop_iso(strm->parent, strm);
strm->parent->device_dead = 1;
break;
}
case LIBUSB_TRANSFER_CANCELLED:
{
FN_SPEW("EP %02x transfer cancelled\n", xfer->endpoint);
if(strm->dead) {
FN_SPEW("EP %02x transfer cancelled\n", xfer->endpoint);
} else {
// This seems to be a libusb bug on OSX - instead of completing
// the transfer with LIBUSB_TRANSFER_NO_DEVICE, the transfers
// simply come back cancelled by the OS. We can detect this,
// though - the stream should be marked dead if we're
// intentionally cancelling transfers.
if(!strm->parent->device_dead) {
FN_ERROR("Got cancelled transfer, but we didn't request it - device disconnected?\n");
}
strm->parent->device_dead = 1;
}
strm->dead_xfers++;
break;
}
Expand All @@ -476,8 +490,8 @@ static void iso_callback(struct libusb_transfer *xfer)
if (res != 0) {
FN_ERROR("Isochronous transfer resubmission failed after unknown error: %d\n", res);
strm->dead_xfers++;
if (res == LIBUSB_TRANSFER_NO_DEVICE) {
fnusb_stop_iso(strm->parent, strm);
if (res == LIBUSB_ERROR_NO_DEVICE) {
strm->parent->device_dead = 1;
}
}
break;
Expand Down Expand Up @@ -528,22 +542,29 @@ int fnusb_stop_iso(fnusb_dev *dev, fnusb_isoc_stream *strm)
freenect_context *ctx = dev->parent->parent;
int i;

FN_FLOOD("fnusb_stop_iso() called\n");

strm->dead = 1;

for (i=0; i<strm->num_xfers; i++)
libusb_cancel_transfer(strm->xfers[i]);
FN_FLOOD("fnusb_stop_iso() cancelled all transfers\n");

while (strm->dead_xfers < strm->num_xfers) {
FN_FLOOD("fnusb_stop_iso() dead = %d\tnum = %d\n", strm->dead_xfers, strm->num_xfers);
libusb_handle_events(ctx->usb.ctx);
}

for (i=0; i<strm->num_xfers; i++)
libusb_free_transfer(strm->xfers[i]);
FN_FLOOD("fnusb_stop_iso() freed all transfers\n");

free(strm->buffer);
free(strm->xfers);

FN_FLOOD("fnusb_stop_iso() freed buffers and stream\n");
memset(strm, 0, sizeof(*strm));
FN_FLOOD("fnusb_stop_iso() done\n");
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions src/usb_libusb10.h
Expand Up @@ -69,6 +69,7 @@ typedef struct {
typedef struct {
freenect_device *parent; //so we can go up from the libusb userdata
libusb_device_handle *dev;
int device_dead; // set to 1 when the underlying libusb_device_handle vanishes (ie, Kinect was unplugged)
} fnusb_dev;

typedef struct {
Expand Down

0 comments on commit 3f7e959

Please sign in to comment.