Improve callbacks return types

CHANGELOG.md

@@ -5,6 +5,7 @@
  * Fix clock selection [#626](https://github.com/membraneframework/membrane_core/pull/626)
  * Log messages in the default handle_info implementation [#680](https://github.com/membraneframework/membrane_core/pull/680)
  * Fix typespecs in Membrane.UtilitySupervisor [#681](https://github.com/membraneframework/membrane_core/pull/681)
+ * Improve callback return types and group actions types [#702](https://github.com/membraneframework/membrane_core/pull/702) 
 
 ## 1.0.0
  * Introduce `:remove_link` action in pipelines and bins.

lib/membrane/bin/action.ex

@@ -15,7 +15,8 @@ defmodule Membrane.Bin.Action do
 
   By default, component setup ends with the end of `c:Membrane.Bin.handle_setup/2` callback.
   If `{:setup, :incomplete}` is returned there, setup lasts until `{:setup, :complete}`
-  is returned from antoher callback.
+  is returned from antoher callback. You cannot return `{:setup, :incomplete}` from any other
+  callback than `c:Membrane.Bin.handle_setup/2`.
 
   Untils the setup lasts, the component won't enter `:playing` playback.
   """

lib/membrane/core/pipeline/action_handler.ex

@@ -8,11 +8,6 @@ defmodule Membrane.Core.Pipeline.ActionHandler do
   alias Membrane.Core.Parent.LifecycleController
   alias Membrane.Core.Pipeline.State
 
-  @impl CallbackHandler
-  def handle_action({action, _args}, :handle_init, _params, _state) when action != :spec do
-    raise ActionError, action: action, reason: {:invalid_callback, :handle_init}
-  end
-
   @impl CallbackHandler
   def handle_action({:spec, args}, _cb, _params, %State{terminating?: true}) do
     raise Membrane.ParentError,

lib/membrane/element/action.ex

@@ -221,7 +221,7 @@ defmodule Membrane.Element.Action do
   @typedoc """
   This action sets the latency for the element.
 
-  This action is not premitted in callback `c:Membrane.Element.Base.handle_init/2`.
+  This action is permitted only in callback `c:Membrane.Element.Base.handle_init/2`.
   """
   @type latency :: {:latency, latency :: Membrane.Time.non_neg()}
 
@@ -245,27 +245,31 @@ defmodule Membrane.Element.Action do
   @type terminate :: {:terminate, reason :: :normal | :shutdown | {:shutdown, term} | term}
 
   @typedoc """
-  Type that defines a single action that may be returned from element callbacks.
+  Action that can be always returned from each of the callbacks.
+  """
+  @type common_actions ::
+          notify_parent | start_timer | timer_interval | stop_timer | terminate | split
 
-  Depending on element type, callback, current playback and other
-  circumstances there may be different actions available.
+  @typedoc """
+  Actions that can be returned from callbacks when the element is in `playback: :playing` state.
   """
-  @type t ::
-          setup
-          | event
-          | notify_parent
-          | split
+  @type stream_actions ::
+          event
           | stream_format
           | buffer
           | demand
-          | redemand
           | pause_auto_demand
           | resume_auto_demand
-          | forward
-          | start_timer
-          | timer_interval
-          | stop_timer
-          | latency
           | end_of_stream
-          | terminate
+          | redemand
+
+  @typedoc """
+  Type that defines a union of actions that may be returned from most of the callbacks.
+
+  Depending on element type, callback, current playback and other
+  circumstances there may be different actions available.
+  Check the typespec and documentation of particular callbacks
+  for details.
+  """
+  @type t :: common_actions | stream_actions | latency | forward | setup
 end

lib/membrane/element/base.ex

@@ -33,11 +33,6 @@ defmodule Membrane.Element.Base do
   alias Membrane.{Element, Event, Pad}
   alias Membrane.Element.{Action, CallbackContext}
 
-  @typedoc """
-  Type that defines all valid return values from most callbacks.
-  """
-  @type callback_return :: {[Action.t()], Element.state()}
-
   @doc """
   Callback invoked on initialization of element.
 
@@ -48,7 +43,7 @@ defmodule Membrane.Element.Base do
   By default, it converts the `opts` struct to a map and sets them as the element's state.
   """
   @callback handle_init(context :: CallbackContext.t(), options :: Element.options()) ::
-              callback_return
+              {[Action.common_actions() | Action.latency()], Element.state()}
 
   @doc """
   Callback invoked on element startup, right after `c:handle_init/2`.
@@ -59,7 +54,7 @@ defmodule Membrane.Element.Base do
   @callback handle_setup(
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) :: {[Action.common_actions() | Action.setup()], Element.state()}
 
   @doc """
   Callback invoked when bin switches the playback to `:playing`.
@@ -71,7 +66,7 @@ defmodule Membrane.Element.Base do
   @callback handle_playing(
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) :: {[Action.common_actions() | Action.stream_actions()], Element.state()}
 
   @doc """
   Callback invoked when element receives a message that is not recognized
@@ -84,7 +79,9 @@ defmodule Membrane.Element.Base do
               message :: any(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) ::
+              {[Action.common_actions() | Action.stream_actions() | Action.setup()],
+               Element.state()}
 
   @doc """
   Callback that is called when new pad has beed added to element. Executed
@@ -97,7 +94,9 @@ defmodule Membrane.Element.Base do
               pad :: Pad.ref(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) ::
+              {[Action.common_actions() | Action.stream_actions() | Action.setup()],
+               Element.state()}
 
   @doc """
   Callback that is called when some pad of the element has beed removed. Executed
@@ -110,7 +109,9 @@ defmodule Membrane.Element.Base do
               pad :: Pad.ref(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) ::
+              {[Action.common_actions() | Action.stream_actions() | Action.setup()],
+               Element.state()}
 
   @doc """
   Callback that is called when event arrives.
@@ -124,7 +125,7 @@ defmodule Membrane.Element.Base do
               event :: Event.t(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) :: {[Action.common_actions() | Action.stream_actions()], Element.state()}
 
   @doc """
   Callback invoked upon each timer tick. A timer can be started with `Membrane.Element.Action.start_timer`
@@ -134,7 +135,9 @@ defmodule Membrane.Element.Base do
               timer_id :: any,
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) ::
+              {[Action.common_actions() | Action.stream_actions() | Action.setup()],
+               Element.state()}
 
   @doc """
   Callback invoked when a message from the parent is received.
@@ -144,7 +147,9 @@ defmodule Membrane.Element.Base do
               notification :: Membrane.ParentNotification.t(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: callback_return
+            ) ::
+              {[Action.common_actions() | Action.stream_actions() | Action.setup()],
+               Element.state()}
 
   @doc """
   Callback invoked when element is removed by its parent.
@@ -155,7 +160,8 @@ defmodule Membrane.Element.Base do
               context :: CallbackContext.t(),
               state :: Element.state()
             ) ::
-              callback_return()
+              {[Action.common_actions() | Action.stream_actions() | Action.setup()],
+               Element.state()}
 
   @doc """
   A callback for constructing struct. Will be defined by `def_options/1` if used.

lib/membrane/element/with_input_pads.ex

@@ -25,16 +25,25 @@ defmodule Membrane.Element.WithInputPads do
               stream_format :: Membrane.StreamFormat.t(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: Membrane.Element.Base.callback_return()
-
+            ) ::
+              {[
+                 Membrane.Element.Action.common_actions()
+                 | Membrane.Element.Action.stream_actions()
+                 | Membrane.Element.Action.forward()
+               ], Element.state()}
   @doc """
   Callback invoked when element receives `Membrane.Event.StartOfStream` event.
   """
   @callback handle_start_of_stream(
               pad :: Pad.ref(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: Membrane.Element.Base.callback_return()
+            ) ::
+              {[
+                 Membrane.Element.Action.common_actions()
+                 | Membrane.Element.Action.stream_actions()
+                 | Membrane.Element.Action.forward()
+               ], Element.state()}
 
   @doc """
   Callback invoked when the previous element has finished processing via the pad,
@@ -44,7 +53,12 @@ defmodule Membrane.Element.WithInputPads do
               pad :: Pad.ref(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: Membrane.Element.Base.callback_return()
+            ) ::
+              {[
+                 Membrane.Element.Action.common_actions()
+                 | Membrane.Element.Action.stream_actions()
+                 | Membrane.Element.Action.forward()
+               ], Element.state()}
 
   @doc """
   Callback that is called when buffer should be processed by the Element.
@@ -59,7 +73,12 @@ defmodule Membrane.Element.WithInputPads do
               buffer :: Buffer.t(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: Membrane.Element.Base.callback_return()
+            ) ::
+              {[
+                 Membrane.Element.Action.common_actions()
+                 | Membrane.Element.Action.stream_actions()
+                 | Membrane.Element.Action.forward()
+               ], Element.state()}
 
   @optional_callbacks handle_buffer: 4, handle_stream_format: 4
 

lib/membrane/element/with_output_pads.ex

@@ -37,7 +37,11 @@ defmodule Membrane.Element.WithOutputPads do
               unit :: Buffer.Metric.unit(),
               context :: CallbackContext.t(),
               state :: Element.state()
-            ) :: Membrane.Element.Base.callback_return()
+            ) ::
+              {[
+                 Membrane.Element.Action.common_actions()
+                 | Membrane.Element.Action.stream_actions()
+               ], Element.state()}
 
   @optional_callbacks handle_demand: 5
 

lib/membrane/pipeline.ex

@@ -106,15 +106,15 @@ defmodule Membrane.Pipeline do
   By default, it converts the `opts` to a map if they're a struct and sets them as the pipeline state.
   """
   @callback handle_init(context :: CallbackContext.t(), options :: pipeline_options) ::
-              callback_return()
+              {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when pipeline is requested to terminate with `terminate/2`.
 
   By default, it returns `t:Membrane.Pipeline.Action.terminate/0` with reason `:normal`.
   """
   @callback handle_terminate_request(context :: CallbackContext.t(), state) ::
-              callback_return()
+              {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked on pipeline startup, right after `c:handle_init/2`.
@@ -126,7 +126,7 @@ defmodule Membrane.Pipeline do
               context :: CallbackContext.t(),
               state
             ) ::
-              callback_return
+              {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when pipeline switches the playback to `:playing`.
@@ -136,7 +136,7 @@ defmodule Membrane.Pipeline do
               context :: CallbackContext.t(),
               state
             ) ::
-              callback_return
+              {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when a child removes its pad.
@@ -151,7 +151,7 @@ defmodule Membrane.Pipeline do
               pad :: Pad.ref(),
               context :: CallbackContext.t(),
               state :: state
-            ) :: callback_return
+            ) :: {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when a notification comes in from a child.
@@ -163,7 +163,7 @@ defmodule Membrane.Pipeline do
               element :: Child.name(),
               context :: CallbackContext.t(),
               state
-            ) :: callback_return
+            ) :: {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when pipeline receives a message that is not recognized
@@ -177,7 +177,7 @@ defmodule Membrane.Pipeline do
               context :: CallbackContext.t(),
               state
             ) ::
-              callback_return
+              {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when a child element starts processing stream via given pad.
@@ -189,7 +189,7 @@ defmodule Membrane.Pipeline do
               pad :: Pad.ref(),
               context :: CallbackContext.t(),
               state
-            ) :: callback_return
+            ) :: {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when a child element finishes processing stream via given pad.
@@ -201,7 +201,7 @@ defmodule Membrane.Pipeline do
               pad :: Pad.ref(),
               context :: CallbackContext.t(),
               state
-            ) :: callback_return
+            ) :: {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when children of `Membrane.ChildrenSpec` are started.
@@ -212,7 +212,7 @@ defmodule Membrane.Pipeline do
               children :: [Child.name()],
               context :: CallbackContext.t(),
               state
-            ) :: callback_return
+            ) :: {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked upon each timer tick. A timer can be started with `Membrane.Pipeline.Action.start_timer`
@@ -222,7 +222,7 @@ defmodule Membrane.Pipeline do
               timer_id :: any,
               context :: CallbackContext.t(),
               state
-            ) :: callback_return
+            ) :: {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when crash of the crash group happens.
@@ -234,7 +234,7 @@ defmodule Membrane.Pipeline do
               group_name :: Child.group(),
               context :: CallbackContext.t(),
               state
-            ) :: callback_return
+            ) :: {[Action.common_actions()], state()}
 
   @doc """
   Callback invoked when pipeline is called using a synchronous call.
@@ -247,7 +247,7 @@ defmodule Membrane.Pipeline do
               context :: CallbackContext.t(),
               state
             ) ::
-              callback_return
+              {[Action.common_actions() | Action.reply()], state()}
 
   @optional_callbacks handle_init: 2,
                       handle_setup: 2,