-
Notifications
You must be signed in to change notification settings - Fork 508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent blocking when calling uvc_stop_streaming #59
base: master
Are you sure you want to change the base?
Conversation
Applied the patches suggested by @saki4510t at issue libuvc#16.
@@ -644,6 +675,11 @@ void LIBUSB_CALL _uvc_stream_callback(struct libusb_transfer *transfer) { | |||
|
|||
if ( strmh->running && resubmit ) | |||
libusb_submit_transfer(transfer); | |||
|
|||
if ( strmh->running && resubmit ) | |||
libusb_submit_transfer(transfer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a duplicate of line 677.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, missed that.
if ( strmh->running && resubmit ) | ||
libusb_submit_transfer(transfer); | ||
else | ||
_uvc_delete_transfer(transfer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function seems to be a copy of the code in the LIBUSB_TRANSFER_CANCELLED / LIBUSB_TRANSFER_ERROR / LIBUSB_TRANSFER_NO_DEVICE case of the switch statement above. That code above can free the transfer, which means the first line of _uvc_delete_transfer, uvc_stream_handle_t *strmh = transfer->user_data;
, will be accessing free'd memory.
free(strmh->transfers[i]->buffer); | ||
libusb_free_transfer(strmh->transfers[i]); | ||
strmh->transfers[i] = NULL; | ||
} | ||
}*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting out the error handling looks wrong.
I need to study the code a little bit more. I'm interested in having this issue fixed. |
This reverts commit 939fac4.
…unning, just free it.
Ok, I've figured it out. So i just surrounded the |
This patch is not solving the problem on Android UPDATE: I've also tried the uvc_stream_start + uvc_stream_get_frame path without using the callback, but it hangs on uvc_stream_stop occasionally |
} else { | ||
/* This is an isochronous mode transfer, so each packet has a payload transfer */ | ||
int packet_id; | ||
if ( strmh->running ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding this if-test and then indenting the following logic, you could instead just make this if !strmh->running break;
(which will also solve the merge conflict I believe.)
Commit 4b6aa9f (was part of #110 not sure where it ended up) solves deadlock/double-free on android during uvc_stop_streaming() for me. Cleaned up patch can be found here: icona-swlab@268ae9d |
Applied the patches suggested by @saki4510t at issue #16.