zwopple / PocketSocket

Objective-C websocket library for building things that work in realtime on iOS and OS X.
Other
414 stars 129 forks source link

Make streams inherit networkServiceType from NSURLRequest #46

Closed shaneomack91 closed 8 years ago

shaneomack91 commented 8 years ago

This allows, for example, to create VoIP sockets by setting networkServiceType as follows:

NSMutableURLRequest *req = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:[command.arguments objectAtIndex:1]]];

req.networkServiceType = NSURLNetworkServiceTypeVoIP;

ws = [PSWebSocket clientSocketWithRequest:req];
robertjpayne commented 8 years ago

@shaneomack91 I'm not sure I really want to support this, using "VOIP" to keep a websocket open is bad form and frankly if you get the entitlement capability pass app store review that doesn't mean you should do it.

Doing so drastically decreasing iOS battery life as your app is kept alive, and it's network connections kept alive forever. It's akin to how Facebook got caught playing a silent audio file to keep the app running in the background.

Do you have any legitimate reason to keep the socket open indefinitely in the background over just using the standard background activity API's or push notifications instead?

robertjpayne commented 8 years ago

@shaneomack91 for what it's worth you can already do this:

PSWebSocket *socket = [PSWebSocket clientSocketWithRequest:…];
[socket setStreamProperty:XXX forKey:];
[socket open];

You have to set the property before you open it…

shaneomack91 commented 8 years ago

I think you're missing the point that some developers (myself included) may wish to keep a WebSocket open to develop, you know, a VOIP app.. lol

Also, the entire point of VoIP mode is that the OS takes control of the socket and suspends your app, until incoming data is received, thus saving battery life. Playing a silent audio file on the other hand will drain battery life like crazy at it is constantly "playing" samples on the devices audio hardware, as well as keeping the app itself running.

On Friday, 22 April 2016, Robert Payne notifications@github.com wrote:

@shaneomack91 I'm not sure I really want to support this, using "VOIP" to keep a websocket open is bad form and frankly if you get the entitlement capability pass app store review that doesn't mean you should do it.

Doing so drastically decreasing iOS battery life as your app is kept alive, and it's network connections kept alive forever. It's akin to how Facebook got caught playing a silent audio file to keep the app running in the background.

Do you have any legitimate reason to keep the socket open indefinitely in the background over just using the standard background activity API's or push notifications instead?

β€” You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub< https://ci4.googleusercontent.com/proxy/rJSKQDqE-OGnkwd-RqpG13DXPOM15Mv3eiqKM4Pp8NAB7i8Q5o_3fggUBftON0-q0cc9Hmc49GHpz0T1dN6x5Gl5xaZncO7Uxzz_77YM9_pwDAAHu7-LKPuW9M1qF182F7GnHfyz1fGdnezRidVbhLee7Ocl1Q=s0-d-e1-ft#https://github.com/notifications/beacon/ABhL74n5Ku8edUp_ZV3Vyog3RplQ1B6Bks5p6E0fgaJpZM4IMBge.gif

robertjpayne commented 8 years ago

@shaneomack91 I may be a bit naive here but I can't help but think using a websocket is a terrible way to implement VOIP. It'd be like trying to build video streaming over websockets just a bad idea?

The capability is there already with the stream properties anyways.

If you're targeting iOS 8.0+ I'd strongly recommend you look at PKPushTypeVoIP and PushKit in general as it's a far more efficient way to implement VOIP connection negotiation without having to keep a socket open which will save your user's battery life heaps πŸ‘

shaneomack91 commented 8 years ago

WebSocket is good for implementing a "control channel", where the actual voice/video data is sent over a RTP/UDP/whatever connection. - Said control channel is the only thing that iOS would need to keep alive.

That said, the particular app I am developing is currently using the WebSocket itself for the Opus-encoded audio data, and works well enough for what it does (rather than a continuous stream, it streams only when the user is holding a "Push-to-talk" key).

I will eventually add a UDP "layer" to this but I am perfecting the WebSocket-only side first, so that it is always available as a fallback if the end-users mobile network doesn't allow UDP.

On 22 April 2016 at 07:35, Robert Payne notifications@github.com wrote:

@shaneomack91 https://github.com/shaneomack91 I may be a bit naive here but I can't help but think using a websocket is a terrible way to implement VOIP. It'd be like trying to build video streaming over websockets just a bad idea?

The capability is there already with the stream properties anyways.

β€” You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/zwopple/PocketSocket/pull/46#issuecomment-213388535

robertjpayne commented 8 years ago

@shaneomack91 makes sense, I had a quick look at the commit again. It's sort of weird because NSURLNetworkServiceTypeVoIP is fine but kCFStreamNetworkServiceTypeVoIP is deprecated in iOS 9.0.

AS such can you try just using the setStreamProperty:forKey: API and see if it's adequate? I have no idea what NSURLSession is potentially doing with NSURLRequests when that network service type is set. It may also get deprecated in iOS 10 unsure.

shaneomack91 commented 8 years ago

I changed my commit to use the newer APIs, they should not be depreceated.

I am aware of PushKit and in fact I am also using it to start up the app when it is closed and and alert is sent. But PushKit does not keep the app running, which for my app is actually necessary because a dispatcher can send out an alert, then there will be people talking on their radios for half an hour or more afterwards and our users want to be able to hear this.

robertjpayne commented 8 years ago

@shaneomack91 np! Didn't realise there was non deprecated NS* version. Merging this now πŸ‘

shaneomack91 commented 8 years ago

Thanks, I submitted this PR because this is how the other Objective-C/iOS WebSocket libraries do the same thing, and frankly, this library is way more reliable than they are.

SocketRocket after a while stops processing incoming data, then bunches it all together at a later time.

JetFire uses spinlocks, which uses 100% CPU and will kill the users battery in no time, not to mention being horribly slow (1-2 seconds for a packet to be sent and to receive the response)

PocketSocket has neither of these problems, I left my app running (with these VoIP mode changes) overnight, in the background, and in the morning it was still connected and ready to rock.

robertjpayne commented 8 years ago

@shaneomack91 thanks for the kind words! I created and opensourced this for those exact reasons!