Send end of stream, even if there was no start of stream before #577
Conversation
FelonEkonom
force-pushed
the
eos-without-start-of-stream
branch
from
9be99a2
to
c2f97b9
Compare
lib/membrane/bin/callback_context.ex
Outdated
| @@ -23,6 +26,7 @@ defmodule Membrane.Bin.CallbackContext do | |||
| :utility_supervisor => Membrane.UtilitySupervisor.t(), | |||
| optional(:pad_options) => map(), | |||
| optional(:members) => [Membrane.Child.name()], | |||
| optional(:crash_initiator) => Membrane.Child.name() | |||
| optional(:crash_initiator) => Membrane.Child.name(), | |||
| optional(:preceded_by_start_of_stream?) => boolean() | |||
There was a problem hiding this comment.
| optional(:preceded_by_start_of_stream?) => boolean() | |
| optional(:start_of_stream_received?) => boolean() |
| defp stream_event_to_callback_context(%Events.StartOfStream{}, _pad_ref), | ||
| do: &CallbackContext.from_state/1 | ||
|
|
||
| defp stream_event_to_callback_context(%Events.EndOfStream{}, pad_ref) do | ||
| &CallbackContext.from_state(&1, | ||
| preceded_by_start_of_stream?: PadModel.get_data!(&1, pad_ref, :start_of_stream?) | ||
| ) | ||
| end | ||
|
|
||
| defp stream_event_to_parent_message_opts(%Events.StartOfStream{}, _pad_ref, _state), do: [] | ||
|
|
||
| defp stream_event_to_parent_message_opts(%Events.EndOfStream{}, pad_ref, state), | ||
| do: [preceded_by_start_of_stream?: PadModel.get_data!(state, pad_ref, :start_of_stream?)] |
There was a problem hiding this comment.
extracting that to separate functions is overkill IMO
There was a problem hiding this comment.
I have extracted it to the separate private functions, because without it do_exec_handle_event was a long serie of statements like
case event_type do
StartOfStream ->
this
EndOfStream ->
that
endThere was a problem hiding this comment.
But actually, when I look at it now, I'd remove handle_special_event and move its logic to the corresponding do_handle_event clauses. I'd also call check_syncs from do_handle_event clause that matches on start of stream.
| @@ -1,7 +1,7 @@ | |||
| defmodule Membrane.Core.Bin.CallbackContext do | |||
| @moduledoc false | |||
|
|
|||
| @type optional_fields :: [pad_options: map()] | |||
| @type optional_fields :: [pad_options: map()] | [preceded_by_start_of_stream?: boolean()] | |||
There was a problem hiding this comment.
seems not changed in Membrane.Core.Pipeline.CallbackContext
|
|
||
| state | ||
| end | ||
|
|
||
| defp do_exec_handle_event(pad_ref, event, params, state) do | ||
| :ok = check_sync(event, state) |
There was a problem hiding this comment.
this is only relevant for start of stream
| with %Events.EndOfStream{} <- event, | ||
| true <- PadModel.get_data!(state, pad_ref, :end_of_stream?) do | ||
| Membrane.Logger.debug("Ignoring end of stream as it has already arrived before") | ||
| state |
There was a problem hiding this comment.
let's handle that in the do_handle_event clause for EndOfStream
Related Jira ticket: https://membraneframework.atlassian.net/browse/MC-211