Allow to record with TCP transport (interleaved mode) #52
Conversation
lib/membrane_rtsp/server/logic.ex
Outdated
| @@ -176,10 +180,20 @@ defmodule Membrane.RTSP.Server.Logic do | |||
| {response, handler_state} = | |||
| state.request_handler.handle_record(state.incoming_media, state.request_handler_state) | |||
|
|
|||
| tcp_interleaved_mode? = | |||
| state.transport_opts[:transport] == :TCP && state.transport_opts[:mode] == :record | |||
There was a problem hiding this comment.
Do we really need to check for transport[:mode], the fact that this method is called means it wants to record.
lib/membrane_rtsp/server/logic.ex
Outdated
| else | ||
| {response, %{state | request_handler_state: handler_state}} | ||
| {response, | ||
| %{state | request_handler_state: handler_state, recording?: tcp_interleaved_mode?}} |
There was a problem hiding this comment.
I think recording? here should be false because the request failed.
lib/membrane_rtsp/server/conn.ex
Outdated
| end | ||
|
|
||
| @impl true | ||
| def handle_info({:rtsp, %Request{} = rtsp_request}, state) do |
There was a problem hiding this comment.
The handler should just send the raw rtsp message, no need to parse it beforehand.
There was a problem hiding this comment.
The problem is to extract the message from the raw TCP stream. How do we know that the RTSP message we are building is done? Is match on "\r\n\r\n" really enough?
There was a problem hiding this comment.
That's the handler responsibility so he can do the parsing however it wants. You can check here for an example on how it's done on the client side.
However we still need to address the following issue, when an rtsp message is sent to the server, it should parse it and then depending on the returned result, we may need to return control to the server. For example when a client send GET_PARAMETER we can just parse it and send the response, however if it sends PAUSE, we need to start listening for rtsp messages in the server, since the handler is no longer responsible for parsing them.
@impl true
def handle_info({:rtsp, raw_rtsp_request}, state) do
with {:ok, request} <- Request.parse(raw_rtsp_request) do
case Logic.process_request(request, state) do
%Logic.State{recording?: true} = state ->
{:noreply, state}
state ->
handle_continue(:process_client_requests, state)
end
end
endI think it's fine to raise in case the rtsp message is not valid.
Anyway the rtsp session (tcp socket) should only be closed when the client close it even TEARDOWN should not close it.
lib/membrane_rtsp/server/conn.ex
Outdated
| end | ||
|
|
||
| @impl true | ||
| def handle_info({:rtsp, %Request{} = rtsp_request}, state) do |
There was a problem hiding this comment.
I think we can delete this.
lib/membrane_rtsp/server/conn.ex
Outdated
| end | ||
|
|
||
| defp do_process_client_requests(state, timeout) do | ||
| with {:ok, request} <- get_request(state.socket, timeout) do | ||
| case Logic.process_request(request, state) do | ||
| %{session_state: :recording} = state -> | ||
| %Logic.State{recording?: true} = state -> |
There was a problem hiding this comment.
looking at the field recording?, it's confusing, what about naming it recording_with_tcp? ??
There was a problem hiding this comment.
Yeap, I think recording? is confusing in this context.
No description provided.