[Development] Qt 5.3 Feature freeze is coming quite soon...

[Kurt Pattyn]

On 17 Jan 2014, at 10:30, Simon Hausmann <simon.hausmann at digia.com> wrote:

> 
> (removing releasing and other dev subscribed folks from CC)
> 
> On Thursday 16. January 2014 12.51.28 Kurt Pattyn wrote:
>> I would like to propose the QtWebSockets module as a new feature for Qt 5.3
>> (see https://qt.gitorious.org/qtplayground/websockets/source/master)
>> 
>> There are a number of requests in Jira:
>> https://bugreports.qt-project.org/issues/?jql=labels%20%3D%20websockets
>> asking to include web socket functionality in Qt, so maybe it is a good
>> time to include it now. Besides that, EnginIo could make use of this module
>> as well (currently it uses an own implementation of web sockets).
>> 
>> QtWebSockets is fully RFC6455 compliant (see
>> http://tools.ietf.org/html/rfc6455) and is successfully tested against the
>> latest Autobahn TestSuite. Performance is OK as well (test report is not
>> included because of too big).
>> 
>> There is a C++ interface as well as a QML module.
>> The C++ API consists of the following classes:
>> QWebSocket
>> QWebSocketServer
>> QWebSocketProtocol
>> 
>> The QML API consists of the following component:
>> WebSocket (included in the Qt.WebSockets 1.0 module).
>> 
>> I see 2 options: either add the functionality to the QtNetwork module, or
>> add it as a Qt add-on. Maybe adding it to QtNetwork would be a ‘natural’
>> place: QWebSocket would then live besides QTcpSocket, QWebSocketServer
>> would live besides QTcpServer.
>> 
>> The module compiles on all reference platforms, has automated unit tests and
>> manual unit tests (against Autobahn TestSuite), is fully documented, and
>> has a number of examples. The module is included in the CI system of Qt
>> (see http://testresults.qt-project.org/ci/WebSockets_master_Integration/).
>> 
>> 
>> Please raise your votes.
> 
> I think it would be a great feature to have in Qt.
> 
> I've had a brief look at the API and I have a few comments:
> 
> * qwebsocketprotocol.h:
>   * V_Unknow should probably be V_Unknow_n_ :)
>   * CC_GOING_AWAY, etc: This should probably use camel casing instead of all upper case, in order to be consistent with the rest of the Qt API.
Agree.

>   * I suggest to remove inline Version currentVersion() { return V_LATEST; } - the function adds no value IMHO as the latest enum value will always be that.
Or maybe make it an ‘outline’ function, in case there comes a newer version of web sockets.
> 
> * qwebsocket.h:
> 
>  * qint64 QWebSocket::write(const char *message)
>    This function appears to send "message" as a text message and expects utf-8 encoding. But unfortunately I think it looks misleading because QIODevice::write has the same signature and means binary content - no assumption about encodings are done. I suggest to rename the
> method to writeTextMessage(const QString &) and just writeBinaryMessage(const QByteArray &) - without any const char * overloads. It makes it very easy to understand the client code when reading it ("ahh, it's sending a text message" or "ahh, it's sending a binary message”).
Indeed. Is is better to remove the (const char *) methods as this can be confusing. I am not sure whether the functions should be renamed to writeTextMessage and writeBinaryMessage. I am following the QIODevice API here.

> 
>   * The API appears to miss a complete the notion of fragments. writeMessage appears to always send a frame, but on the receiving end the concept of fragments appears - this looks inconsistent. I'll elaborate a bit more about that separately.
The reason the textFrameReceived and binaryFrameReceived signals are there, is to let the receiving application start processing the message as soon as a fragment arrives. This can be interesting in case of large payloads.
> 
> I can think of two different ways of using web sockets:
> 
> 1) Sending/receiving messages of a given protocol
> 2) Sending/receiving streams
> 
> It appears that the web socket protocol encapsulates both, with the notion of messages consisting of a variable (potentially unlimited) number of fragments. I think it is important to make both use-cases very easy. When I'm implementing
> a message based protocol (for example chat client), I really want the underlying framework to deal with fragments and only deliver entire frames to me. When implementing for example a video streaming service, I need full control (on both ends)
> over fragmentation.


> And to make it worse, it seems that over the same connection I could be sending a stream (variable fragments of the same frame) intermixed with single fragments. Is my interpretation of the spec correct here?
With web sockets you can intermix text and binary messages, but you have no control over the ‘fragments’ (frames in web socket lingo). These frames are comparable to TCP or UDP packets. Web sockets is splitting ‘large’ messages into frames ‘under the hood’, where the first frame indicates that it is the first frame, subsequent frames indicate that they are continuation frames and the last frame indicates it is the final one.
If you want to send small payloads, then you need to send them as a whole message that is no larger than a single frame.

To make the streaming case work, there should be a mechanism to indicate that a ‘fragment’ is the first one, a continuation ‘fragment’ or the last ‘fragment'. Of course, this could be encapsulated into a streaming API.
But, this would not work in web browsers, as browsers take whole messages at once. Also, the current web socket QML component is modelled after the web socket API, and hence would not be able to consume these streams.
So, I’d rather not change this behaviour.
For an example of live video streaming using web sockets, see: http://phoboslab.org/log/2013/09/html5-live-video-streaming-via-websockets.

Maybe it could be a good idea to be able to configure the frame size. But, I suggest to leave this one out for now.

> 
> 
> Either way what I'm missing from the API is making these two use-cases easier. Perhaps it would make sense to offer convenience, such as a boolean property to indicate that all text frames should only be signalled (textFrameReceived) as entire
> frame instead of fragments.
Well, if you don’t connect to the signal you won’t receive them, or do I miss something?

Cheers,

Kurt
> 
> I have more thoughts on this, but let's start the discussion here maybe :)
> 
> Simon