Skip to content
This repository has been archived by the owner on Oct 20, 2024. It is now read-only.

Make streams inherit networkServiceType from NSURLRequest #46

Merged
merged 2 commits into from
Apr 22, 2016

Conversation

shaneomack91
Copy link

@shaneomack91 shaneomack91 commented Apr 20, 2016

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];

….g. creation of VoIP background mode streams
@robertjpayne
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Contributor

robertjpayne commented Apr 22, 2016

@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
Copy link
Author

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
#46 (comment)

@robertjpayne
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Contributor

@shaneomack91 np! Didn't realise there was non deprecated NS* version. Merging this now 👍

@robertjpayne robertjpayne merged commit b5b9253 into zwopple:master Apr 22, 2016
@shaneomack91
Copy link
Author

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
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants