-
Notifications
You must be signed in to change notification settings - Fork 179
Fix iOS connections dropping and some overhaul #184
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
Conversation
strncpy() does not guarantee zero-termination! Set the last byte explicitly. Not a serious problem with constant strings, but still better to avoid: sprintf() and strcpy() do not check the target buffer size. atoi() usage is discouraged; port numbers are unsigned, so use strtoul().
provide access to usbmuxd_send()/usbmuxd_recv() via SendRecviOS() in usb.c select which SendRecv() function to use in connection.c link to libusbmuxd-2.0
|
Thanks, will go over this week as time permits. FWIW the existing clients have been around a while, the larger plan is to rewrite everything with a cleaner cross-platform implementation. |
| JPEG = -lturbojpeg | ||
| SRC = src/connection.c src/settings.c src/decoder*.c src/av.c src/usb.c src/queue.c | ||
| USBMUXD = -lusbmuxd-2.0 | ||
| USBMUXD = -lusbmuxd |
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 reverts the first commit to use -2.0. Does that mean it's just a matter of using usbmuxd_send/recv to fix the connection reset problem?
Looking at the implementation of those functions, it's just a matter of the socket being nonblocking. I'm surprised the problem only shows up on Arch in that case 🤔
Can we just remove the NONBLOCK flag from the usbmuxd socket and leave everything as-is?
Ot better yet, convert all other sockets to NONBLOCK and not have the av threads locked in syscalls (would also address #180).
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.
actually I didn't know how usbmuxd does its thing. I'll look into it and implement your suggestion if it works :)
Makefile
Outdated
| LIBAV = `pkg-config --libs --cflags libswscale libavutil` | ||
| LIBS = -lspeex -lasound -lpthread -lm | ||
| JPEG = -I$(JPEG_INCLUDE) $(JPEG_LIB)/libturbojpeg.a | ||
| JPEG = -lturbojpeg |
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.
The dynamic linking has been brought up many times before. This makes turbojpeg an install time dependency, and there is too much fragmentation across distros (for all packages). If/when the client is re-written, it can use the generic v4l2loopback and can potentially be vendored as a .deb.
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.
ah, this makes sense. It just worked so well on my test candidates, but they're rather current I admit.
I guess the only reason not to use upstream v4l2loopback is vendor fragmentation, too?
I'll revert the commit, if anyone wants to link dynamically they can just use make JPEG=-lturbojpeg.
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.
v4l2loopback used to be sparsely supported, but now I think its well supported. The droidcam fork mostly exists for historical and UX reasons.
turbojpeg is a challenge still.
set correct socket flags in CheckiOSDevices() remove SendRecvUSB() switching remove SendRecviOS()
This reverts commit 42b9e5d.
|
You were right, removing the NONBLOCK flag did the trick! I implemented this and removed the usbmuxd_{send,recv} stuff. |
|
I've rebased and slightly re-arranged a few things into this branch Everything seems to be working, I just need to test iOS usb connections a little and the branch will get merged into master. |
|
Nice! Thanks for your work on this. I've had a look at your branch and it works fine for me 😃 |
The main commit is 303d6b5, it fixes #97 (connection drop on Arch Linux).
The rest are minor improvements for code quality; I would highly recommend 272541b for improved string handling.
Re-implementation of the queue, the change in libjpegturbo linking and include cleanup are debatable, but none of the others depend on them, so they can easily be reverted. Tell me what you think.
Tested on: