Add releaseStream support and fix FCPublish call order

[levs42]

Description & motivation

This PR allows the library users to be able to call code before createStream. For example, to call releaseStream which is supported by Wowza. New state is required because .initialized is the default state and when the connection is successful, changing the ready state here doesn't lead to the delegate notification call. In addition, createStream is wrapped into a lock queue, so library users have time to perform extra calls on the same queue before the create stream execution. It also adds an additional send method to allow connection.call method call from the outside.

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

[shogo4405]

Upon reconsideration of FCPublish/FCUnpublish, it seems more natural for them to exist in RTMPConnection rather than RTMPStream. I believe it would be more appropriate to implement them properly on the RTMPConnection side rather than modifying RTMPStream.

Even if it were to be merged, I have no plans to accept it, as the relevant code itself will be removed in the next version.

[shogo4405]

[IMO]
RTMPConneciton.call is already public, so I don't feel the need for implementation on the HaishinKit side. We expect it to be defined in the importing side's extension or RTMPStream extension.

[levs42]

Sounds good. connection is private, but it's injected into the RTMPStream so it can be used from the outside.

[shogo4405]

With the increase of "uninitialized," the number of states in readyState has increased, making it difficult to foresee and maintain. I also think "closed = uninitialized" is synonymous.

[levs42]

Considering that FCPublish/FCUnpublish will move into RTMPConnection, what could be a way to call releaseStream before createStream? I pushed 21a7def with an example so the key can be set from the outside. Initial idea is to subscribe to RTMPConnection.Code.connectSuccess but there is no guarantee about the event listeners dispatch order, so createStream might be executed before releaseStream.

[shogo4405]

If you modify createStream as follows, it ensures that the call to createStream will occur after the releaseStream command is received.

 func createStream(_ stream: RTMPStream) {
        let releaseStreamResponder = RTMPResponder(result: { data -> Void in
                let responder = RTMPResponder(result: { data -> Void in
            guard let id = data[0] as? Double else {
                return
            }
            stream.id = UInt32(id)
            stream.readyState = .open
           })
           call("createStream", responder: responder)
        })
        call(“releaseStream”, responder: releaseStreamResponder, arguments: stream.key)
    }

In modern times, it cannot be simply assumed whether various server-side implementations are correctly implemented or not. I think it would be good to call it like this.

func createStream(_ stream: RTMPStream) {
    // Specify an empty responder to ignore the result of releaseStream.
    let releaseStreamResponder = RTMPResponder(result: { data -> Void in })
    call(“releaseStream”, responder: releaseStreamResponder, arguments: stream.key)
    call(“createStream”, responder: …)
}

[levs42]

I've updated the PR with optional releaseStream call without a responder.

[levs42]

In addition to optional releaseStream call, I moved FCPublish to be called before createStream. I can reference Adobe software and other encoders that encode in this order so it's defacto standard for this undocumented command. In addition, Twitter doesn't respond with onFCPublish when FCPublish is after createStream:

Screenshot

Now it's a breaking change because users have to pass name to connect() method in order to FCPublish to work.

[shogo4405]

[MUST]
I don't want to make such disruptive changes for the purpose of supporting releaseStream. Also, considering viewing or multiple streams, I still think this change is unnecessary.

If I was to do it, I think it would be better to prepare an RTMPConnection subclass or wrapper class, such as RTMPConnectionFMLECompatible, that conforms to the FMLE sequence.

[levs42]

Should I make RTMPConnection and createStream() open to allow subclassing? My goal is to have any kind of solution for this case being supported. It's common and solves platform specific issues while not causing others because all other popular encoders perform it in the mentioned sequence. FMLE app performs it in the following order:

AMF command 'releaseStream'
AMF command 'FCPublish'
AMF command 'createStream'

[shogo4405]

Yes. HaishinKit aims to provide multi-streaming and ingest/playback functionalities. Killing off RTMPConnection functionality or making destructive changes to methods in pursuit of FMLE compatibility would diverge from this objective.

Therefore, I believe it would be appropriate to have a class dedicated to single-stream ingest, inheriting from RTMPConnection, and within that class, call the FMLE-compatible methods.

[shogo4405]

It seems natural for the stream name of releaseStream to be in RTMPStream.
To avoid issues with multi-streaming and playback without inheriting RTMPConnection, this implementation seems necessary.
https://github.com/shogo4405/HaishinKit.swift/pull/1393/files

It might be good to handle errors when there is a contradiction between the publish and fcPublishName, but I don't think it's necessary to be that strict.

[shogo4405]

Close as #1393