Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor callback contexts #504

Merged
merged 9 commits into from
Jan 11, 2023
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Replace `Membrane.Time.round_to_<unit_name>` with `Membrane.Time.as_<unit_name>/2` with second argument equal `:round`. Rename `Membrane.Time.round_to_timebase` to `Membrane.Time.divide_by_timebase/2`. [#494](https://github.com/membraneframework/membrane_core/pull/494)
* Remove `:playback` action. Introduce `:setup` action. [#496](https://github.com/membraneframework/membrane_core/pull/496)
* Add `Membrane.Testing.Pipeline.get_child_pid/2`. [#497](https://github.com/membraneframework/membrane_core/pull/497)
* Make callback contexts to be maps. [#504](https://github.com/membraneframework/membrane_core/pull/504)

## 0.11.0
* Separate element_name and pad arguments in handle_element_{start, end}_of_stream signature [#219](https://github.com/membraneframework/membrane_core/issues/219)
Expand Down
33 changes: 20 additions & 13 deletions lib/membrane/bin.ex
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,30 @@ defmodule Membrane.Bin do
while `handle_init` should be used for things like parsing options, initializing state or
spawning children.
"""
@callback handle_init(context :: CallbackContext.Init.t(), options :: options_t) ::
@callback handle_init(context :: CallbackContext.t(), options :: options_t) ::
callback_return_t()

@doc """
Callback that is called when new pad has been added to bin. Executed
ONLY for dynamic pads.

Context passed to this callback contains additional field `:pad_options`.
"""
@callback handle_pad_added(
pad :: Pad.ref_t(),
context :: CallbackContext.PadAdded.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

@doc """
Callback that is called when some pad of the bin has been removed. Executed
ONLY for dynamic pads.

Context passed to this callback contains additional field `:pad_options`.
"""
@callback handle_pad_removed(
pad :: Pad.ref_t(),
context :: CallbackContext.PadRemoved.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -73,15 +77,15 @@ defmodule Membrane.Bin do
Any long-lasting or complex initialization should happen here.
"""
@callback handle_setup(
context :: CallbackContext.Setup.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

@doc """
Callback invoked when bin switches the playback to `:playing`.
"""
@callback handle_playing(
context :: CallbackContext.Playing.t(),
context :: CallbackContext.t(),
state :: state_t
) ::
callback_return_t
Expand All @@ -92,7 +96,7 @@ defmodule Membrane.Bin do
@callback handle_child_notification(
notification :: Membrane.ChildNotification.t(),
element :: Child.name_t(),
context :: CallbackContext.ChildNotification.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -101,7 +105,7 @@ defmodule Membrane.Bin do
"""
@callback handle_parent_notification(
notification :: Membrane.ParentNotification.t(),
context :: CallbackContext.ParentNotification.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -113,7 +117,7 @@ defmodule Membrane.Bin do
"""
@callback handle_info(
message :: any,
context :: CallbackContext.Info.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -123,7 +127,7 @@ defmodule Membrane.Bin do
@callback handle_element_start_of_stream(
child :: Child.name_t(),
pad :: Pad.ref_t(),
context :: CallbackContext.StreamManagement.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -133,7 +137,7 @@ defmodule Membrane.Bin do
@callback handle_element_end_of_stream(
child :: Child.name_t(),
pad :: Pad.ref_t(),
context :: CallbackContext.StreamManagement.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -142,7 +146,7 @@ defmodule Membrane.Bin do
"""
@callback handle_spec_started(
children :: [Child.name_t()],
context :: CallbackContext.SpecStarted.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -152,7 +156,7 @@ defmodule Membrane.Bin do
"""
@callback handle_tick(
timer_id :: any,
context :: CallbackContext.Tick.t(),
context :: CallbackContext.t(),
state :: state_t
) :: callback_return_t

Expand All @@ -161,7 +165,10 @@ defmodule Membrane.Bin do

By default it returns `t:Membrane.Bin.Action.terminate_t/0` with reason `:normal`.
"""
@callback handle_terminate_request(context :: CallbackContext.TerminateRequest.t(), state_t) ::
@callback handle_terminate_request(
context :: CallbackContext.t(),
state_t
) ::
callback_return_t()

@optional_callbacks handle_init: 2,
Expand Down
23 changes: 23 additions & 0 deletions lib/membrane/bin/callback_context.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
defmodule Membrane.Bin.CallbackContext do
@moduledoc """
Module describing context passed to the `Membrane.Bin` callbacks.
"""

@typedoc """
Type describing context passed to the `Membrane.Bin` callbacks.

Field `:pad_options` is present only in `c:Membrane.Bin.handle_pad_added/3`
and `c:Membrane.Bin.handle_pad_removed/3`.
"""
@type t :: %{
:clock => Membrane.Clock.t(),
:parent_clock => Membrane.Clock.t(),
:pads => %{Membrane.Pad.ref_t() => Membrane.Bin.PadData.t()},
:name => Membrane.Bin.name_t(),
:children => %{Membrane.Child.name_t() => Membrane.ChildEntry.t()},
:playback => Membrane.Playback.t(),
:resource_guard => Membrane.ResourceGuard.t(),
:utility_supervisor => Membrane.UtilitySupervisor.t(),
optional(:pad_options) => map()
}
end
7 changes: 0 additions & 7 deletions lib/membrane/bin/callback_context/child_notification.ex

This file was deleted.

9 changes: 0 additions & 9 deletions lib/membrane/bin/callback_context/crash_group_down.ex

This file was deleted.

7 changes: 0 additions & 7 deletions lib/membrane/bin/callback_context/info.ex

This file was deleted.

6 changes: 0 additions & 6 deletions lib/membrane/bin/callback_context/init.ex

This file was deleted.

9 changes: 0 additions & 9 deletions lib/membrane/bin/callback_context/pad_added.ex

This file was deleted.

8 changes: 0 additions & 8 deletions lib/membrane/bin/callback_context/pad_removed.ex

This file was deleted.

7 changes: 0 additions & 7 deletions lib/membrane/bin/callback_context/parent_notification.ex

This file was deleted.

6 changes: 0 additions & 6 deletions lib/membrane/bin/callback_context/playing.ex

This file was deleted.

6 changes: 0 additions & 6 deletions lib/membrane/bin/callback_context/setup.ex

This file was deleted.

7 changes: 0 additions & 7 deletions lib/membrane/bin/callback_context/spec_started.ex

This file was deleted.

7 changes: 0 additions & 7 deletions lib/membrane/bin/callback_context/stream_management.ex

This file was deleted.

6 changes: 0 additions & 6 deletions lib/membrane/bin/callback_context/terminate_request.ex

This file was deleted.

7 changes: 0 additions & 7 deletions lib/membrane/bin/callback_context/tick.ex

This file was deleted.

6 changes: 2 additions & 4 deletions lib/membrane/core/bin.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule Membrane.Core.Bin do
use GenServer

alias __MODULE__.{ActionHandler, PadController, State}
alias Membrane.Bin.CallbackContext
alias Membrane.Core.Bin.CallbackContext

alias Membrane.Core.{
CallbackHandler,
Expand Down Expand Up @@ -125,13 +125,11 @@ defmodule Membrane.Core.Bin do
}
|> Child.PadSpecHandler.init_pads()

require CallbackContext.Init

state =
CallbackHandler.exec_and_handle_callback(
:handle_init,
ActionHandler,
%{context: &CallbackContext.Init.from_state/1},
%{context: &CallbackContext.from_state/1},
[],
%{state | internal_state: options.user_options}
)
Expand Down
38 changes: 15 additions & 23 deletions lib/membrane/core/bin/callback_context.ex
Original file line number Diff line number Diff line change
@@ -1,29 +1,21 @@
defmodule Membrane.Core.Bin.CallbackContext do
@moduledoc false

use Membrane.Core.CallbackContext,
clock: Membrane.Clock.t(),
parent_clock: Membrane.Clock.t(),
pads: %{Membrane.Pad.ref_t() => Membrane.Bin.PadData.t()},
name: Membrane.Bin.name_t(),
children: %{Membrane.Child.name_t() => Membrane.ChildEntry.t()},
playback: Membrane.Playback.t(),
resource_guard: Membrane.ResourceGuard.t(),
utility_supervisor: Membrane.UtilitySupervisor.t()
@type optional_fields_t :: [pad_options: map()]

@impl true
def extract_default_fields(state, args) do
quote do
[
clock: unquote(state).synchronization.clock,
parent_clock: unquote(state).synchronization.parent_clock,
pads: unquote(state).pads_data,
name: unquote(state).name,
children: unquote(state).children,
playback: unquote(state).playback,
resource_guard: unquote(state).resource_guard,
utility_supervisor: unquote(state).subprocess_supervisor
]
end ++ args
@spec from_state(Membrane.Core.Bin.State.t(), optional_fields_t()) ::
Membrane.Bin.CallbackContext.t()
def from_state(state, optional_fields \\ []) do
Map.new(optional_fields)
|> Map.merge(%{
clock: state.synchronization.clock,
parent_clock: state.synchronization.parent_clock,
pads: state.pads_data,
name: state.name,
children: state.children,
playback: state.playback,
resource_guard: state.resource_guard,
utility_supervisor: state.subprocess_supervisor
})
end
end
15 changes: 5 additions & 10 deletions lib/membrane/core/bin/pad_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@ defmodule Membrane.Core.Bin.PadController do

use Bunch

alias Membrane.Bin.CallbackContext
alias Membrane.{Core, LinkError, Pad}
alias Membrane.Core.Bin.{ActionHandler, State}
alias Membrane.Core.Bin.{ActionHandler, CallbackContext, State}
alias Membrane.Core.{CallbackHandler, Child, Message}
alias Membrane.Core.Child.PadModel
alias Membrane.Core.Element.StreamFormatController
alias Membrane.Core.Parent.{ChildLifeController, Link, SpecificationParser}

require Membrane.Core.Child.PadModel
require Membrane.Core.Message
require Membrane.Bin.CallbackContext.{PadAdded, PadRemoved}
require Membrane.Logger
require Membrane.Pad

Expand Down Expand Up @@ -278,11 +276,10 @@ defmodule Membrane.Core.Bin.PadController do

@spec maybe_handle_pad_added(Pad.ref_t(), Core.Bin.State.t()) :: Core.Bin.State.t()
defp maybe_handle_pad_added(ref, state) do
%{options: pad_opts, direction: direction, availability: availability} =
PadModel.get_data!(state, ref)
%{options: pad_opts, availability: availability} = PadModel.get_data!(state, ref)

if Pad.availability_mode(availability) == :dynamic do
context = &CallbackContext.PadAdded.from_state(&1, options: pad_opts, direction: direction)
context = &CallbackContext.from_state(&1, pad_options: pad_opts)

CallbackHandler.exec_and_handle_callback(
:handle_pad_added,
Expand All @@ -298,15 +295,13 @@ defmodule Membrane.Core.Bin.PadController do

@spec maybe_handle_pad_removed(Pad.ref_t(), Core.Bin.State.t()) :: Core.Bin.State.t()
defp maybe_handle_pad_removed(ref, state) do
%{direction: direction, availability: availability} = PadModel.get_data!(state, ref)
%{availability: availability} = PadModel.get_data!(state, ref)

if Pad.availability_mode(availability) == :dynamic do
context = &CallbackContext.PadRemoved.from_state(&1, direction: direction)

CallbackHandler.exec_and_handle_callback(
:handle_pad_removed,
ActionHandler,
%{context: context},
%{context: &CallbackContext.from_state/1},
[ref],
state
)
Expand Down
Loading