Skip to content

Commit 5c2a831

Browse files
varsillvarsill
authored
Improve callbacks return types (#702)
* Remove restrictions concerning returned actions in `Membrane.Pipeline.handle_init/2`. * Fix a missleading information in docs about returning `:latency` action from `Membrane.Element.Base.handle_init` * Split the callback return types into unions of multiple subtypes
1 parent ebb7702 commit 5c2a831

File tree

9 files changed

+96
-62
lines changed

9 files changed

+96
-62
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* Fix clock selection [#626](https://github.com/membraneframework/membrane_core/pull/626)
66
* Log messages in the default handle_info implementation [#680](https://github.com/membraneframework/membrane_core/pull/680)
77
* Fix typespecs in Membrane.UtilitySupervisor [#681](https://github.com/membraneframework/membrane_core/pull/681)
8+
* Improve callback return types and group actions types [#702](https://github.com/membraneframework/membrane_core/pull/702)
89

910
## 1.0.0
1011
* Introduce `:remove_link` action in pipelines and bins.

‎lib/membrane/bin/action.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ defmodule Membrane.Bin.Action do
1515
1616
By default, component setup ends with the end of `c:Membrane.Bin.handle_setup/2` callback.
1717
If `{:setup, :incomplete}` is returned there, setup lasts until `{:setup, :complete}`
18-
is returned from antoher callback.
18+
is returned from antoher callback. You cannot return `{:setup, :incomplete}` from any other
19+
callback than `c:Membrane.Bin.handle_setup/2`.
1920
2021
Untils the setup lasts, the component won't enter `:playing` playback.
2122
"""

‎lib/membrane/core/pipeline/action_handler.ex

-5
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ defmodule Membrane.Core.Pipeline.ActionHandler do
88
alias Membrane.Core.Parent.LifecycleController
99
alias Membrane.Core.Pipeline.State
1010

11-
@impl CallbackHandler
12-
def handle_action({action, _args}, :handle_init, _params, _state) when action != :spec do
13-
raise ActionError, action: action, reason: {:invalid_callback, :handle_init}
14-
end
15-
1611
@impl CallbackHandler
1712
def handle_action({:spec, args}, _cb, _params, %State{terminating?: true}) do
1813
raise Membrane.ParentError,

‎lib/membrane/element/action.ex

+20-16
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ defmodule Membrane.Element.Action do
221221
@typedoc """
222222
This action sets the latency for the element.
223223
224-
This action is not premitted in callback `c:Membrane.Element.Base.handle_init/2`.
224+
This action is permitted only in callback `c:Membrane.Element.Base.handle_init/2`.
225225
"""
226226
@type latency :: {:latency, latency :: Membrane.Time.non_neg()}
227227

@@ -245,27 +245,31 @@ defmodule Membrane.Element.Action do
245245
@type terminate :: {:terminate, reason :: :normal | :shutdown | {:shutdown, term} | term}
246246

247247
@typedoc """
248-
Type that defines a single action that may be returned from element callbacks.
248+
Action that can be always returned from each of the callbacks.
249+
"""
250+
@type common_actions ::
251+
notify_parent | start_timer | timer_interval | stop_timer | terminate | split
249252

250-
Depending on element type, callback, current playback and other
251-
circumstances there may be different actions available.
253+
@typedoc """
254+
Actions that can be returned from callbacks when the element is in `playback: :playing` state.
252255
"""
253-
@type t ::
254-
setup
255-
| event
256-
| notify_parent
257-
| split
256+
@type stream_actions ::
257+
event
258258
| stream_format
259259
| buffer
260260
| demand
261-
| redemand
262261
| pause_auto_demand
263262
| resume_auto_demand
264-
| forward
265-
| start_timer
266-
| timer_interval
267-
| stop_timer
268-
| latency
269263
| end_of_stream
270-
| terminate
264+
| redemand
265+
266+
@typedoc """
267+
Type that defines a union of actions that may be returned from most of the callbacks.
268+
269+
Depending on element type, callback, current playback and other
270+
circumstances there may be different actions available.
271+
Check the typespec and documentation of particular callbacks
272+
for details.
273+
"""
274+
@type t :: common_actions | stream_actions | latency | forward | setup
271275
end

‎lib/membrane/element/base.ex

+21-15
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ defmodule Membrane.Element.Base do
3333
alias Membrane.{Element, Event, Pad}
3434
alias Membrane.Element.{Action, CallbackContext}
3535

36-
@typedoc """
37-
Type that defines all valid return values from most callbacks.
38-
"""
39-
@type callback_return :: {[Action.t()], Element.state()}
40-
4136
@doc """
4237
Callback invoked on initialization of element.
4338
@@ -48,7 +43,7 @@ defmodule Membrane.Element.Base do
4843
By default, it converts the `opts` struct to a map and sets them as the element's state.
4944
"""
5045
@callback handle_init(context :: CallbackContext.t(), options :: Element.options()) ::
51-
callback_return
46+
{[Action.common_actions() | Action.latency()], Element.state()}
5247

5348
@doc """
5449
Callback invoked on element startup, right after `c:handle_init/2`.
@@ -59,7 +54,7 @@ defmodule Membrane.Element.Base do
5954
@callback handle_setup(
6055
context :: CallbackContext.t(),
6156
state :: Element.state()
62-
) :: callback_return
57+
) :: {[Action.common_actions() | Action.setup()], Element.state()}
6358

6459
@doc """
6560
Callback invoked when bin switches the playback to `:playing`.
@@ -71,7 +66,7 @@ defmodule Membrane.Element.Base do
7166
@callback handle_playing(
7267
context :: CallbackContext.t(),
7368
state :: Element.state()
74-
) :: callback_return
69+
) :: {[Action.common_actions() | Action.stream_actions()], Element.state()}
7570

7671
@doc """
7772
Callback invoked when element receives a message that is not recognized
@@ -84,7 +79,9 @@ defmodule Membrane.Element.Base do
8479
message :: any(),
8580
context :: CallbackContext.t(),
8681
state :: Element.state()
87-
) :: callback_return
82+
) ::
83+
{[Action.common_actions() | Action.stream_actions() | Action.setup()],
84+
Element.state()}
8885

8986
@doc """
9087
Callback that is called when new pad has beed added to element. Executed
@@ -97,7 +94,9 @@ defmodule Membrane.Element.Base do
9794
pad :: Pad.ref(),
9895
context :: CallbackContext.t(),
9996
state :: Element.state()
100-
) :: callback_return
97+
) ::
98+
{[Action.common_actions() | Action.stream_actions() | Action.setup()],
99+
Element.state()}
101100

102101
@doc """
103102
Callback that is called when some pad of the element has beed removed. Executed
@@ -110,7 +109,9 @@ defmodule Membrane.Element.Base do
110109
pad :: Pad.ref(),
111110
context :: CallbackContext.t(),
112111
state :: Element.state()
113-
) :: callback_return
112+
) ::
113+
{[Action.common_actions() | Action.stream_actions() | Action.setup()],
114+
Element.state()}
114115

115116
@doc """
116117
Callback that is called when event arrives.
@@ -124,7 +125,7 @@ defmodule Membrane.Element.Base do
124125
event :: Event.t(),
125126
context :: CallbackContext.t(),
126127
state :: Element.state()
127-
) :: callback_return
128+
) :: {[Action.common_actions() | Action.stream_actions()], Element.state()}
128129

129130
@doc """
130131
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
134135
timer_id :: any,
135136
context :: CallbackContext.t(),
136137
state :: Element.state()
137-
) :: callback_return
138+
) ::
139+
{[Action.common_actions() | Action.stream_actions() | Action.setup()],
140+
Element.state()}
138141

139142
@doc """
140143
Callback invoked when a message from the parent is received.
@@ -144,7 +147,9 @@ defmodule Membrane.Element.Base do
144147
notification :: Membrane.ParentNotification.t(),
145148
context :: CallbackContext.t(),
146149
state :: Element.state()
147-
) :: callback_return
150+
) ::
151+
{[Action.common_actions() | Action.stream_actions() | Action.setup()],
152+
Element.state()}
148153

149154
@doc """
150155
Callback invoked when element is removed by its parent.
@@ -155,7 +160,8 @@ defmodule Membrane.Element.Base do
155160
context :: CallbackContext.t(),
156161
state :: Element.state()
157162
) ::
158-
callback_return()
163+
{[Action.common_actions() | Action.stream_actions() | Action.setup()],
164+
Element.state()}
159165

160166
@doc """
161167
A callback for constructing struct. Will be defined by `def_options/1` if used.

‎lib/membrane/element/with_input_pads.ex

+24-5
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,25 @@ defmodule Membrane.Element.WithInputPads do
2525
stream_format :: Membrane.StreamFormat.t(),
2626
context :: CallbackContext.t(),
2727
state :: Element.state()
28-
) :: Membrane.Element.Base.callback_return()
29-
28+
) ::
29+
{[
30+
Membrane.Element.Action.common_actions()
31+
| Membrane.Element.Action.stream_actions()
32+
| Membrane.Element.Action.forward()
33+
], Element.state()}
3034
@doc """
3135
Callback invoked when element receives `Membrane.Event.StartOfStream` event.
3236
"""
3337
@callback handle_start_of_stream(
3438
pad :: Pad.ref(),
3539
context :: CallbackContext.t(),
3640
state :: Element.state()
37-
) :: Membrane.Element.Base.callback_return()
41+
) ::
42+
{[
43+
Membrane.Element.Action.common_actions()
44+
| Membrane.Element.Action.stream_actions()
45+
| Membrane.Element.Action.forward()
46+
], Element.state()}
3847

3948
@doc """
4049
Callback invoked when the previous element has finished processing via the pad,
@@ -44,7 +53,12 @@ defmodule Membrane.Element.WithInputPads do
4453
pad :: Pad.ref(),
4554
context :: CallbackContext.t(),
4655
state :: Element.state()
47-
) :: Membrane.Element.Base.callback_return()
56+
) ::
57+
{[
58+
Membrane.Element.Action.common_actions()
59+
| Membrane.Element.Action.stream_actions()
60+
| Membrane.Element.Action.forward()
61+
], Element.state()}
4862

4963
@doc """
5064
Callback that is called when buffer should be processed by the Element.
@@ -59,7 +73,12 @@ defmodule Membrane.Element.WithInputPads do
5973
buffer :: Buffer.t(),
6074
context :: CallbackContext.t(),
6175
state :: Element.state()
62-
) :: Membrane.Element.Base.callback_return()
76+
) ::
77+
{[
78+
Membrane.Element.Action.common_actions()
79+
| Membrane.Element.Action.stream_actions()
80+
| Membrane.Element.Action.forward()
81+
], Element.state()}
6382

6483
@optional_callbacks handle_buffer: 4, handle_stream_format: 4
6584

‎lib/membrane/element/with_output_pads.ex

+5-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ defmodule Membrane.Element.WithOutputPads do
3737
unit :: Buffer.Metric.unit(),
3838
context :: CallbackContext.t(),
3939
state :: Element.state()
40-
) :: Membrane.Element.Base.callback_return()
40+
) ::
41+
{[
42+
Membrane.Element.Action.common_actions()
43+
| Membrane.Element.Action.stream_actions()
44+
], Element.state()}
4145

4246
@optional_callbacks handle_demand: 5
4347

‎lib/membrane/pipeline.ex

+13-13
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,15 @@ defmodule Membrane.Pipeline do
106106
By default, it converts the `opts` to a map if they're a struct and sets them as the pipeline state.
107107
"""
108108
@callback handle_init(context :: CallbackContext.t(), options :: pipeline_options) ::
109-
callback_return()
109+
{[Action.common_actions()], state()}
110110

111111
@doc """
112112
Callback invoked when pipeline is requested to terminate with `terminate/2`.
113113
114114
By default, it returns `t:Membrane.Pipeline.Action.terminate/0` with reason `:normal`.
115115
"""
116116
@callback handle_terminate_request(context :: CallbackContext.t(), state) ::
117-
callback_return()
117+
{[Action.common_actions()], state()}
118118

119119
@doc """
120120
Callback invoked on pipeline startup, right after `c:handle_init/2`.
@@ -126,7 +126,7 @@ defmodule Membrane.Pipeline do
126126
context :: CallbackContext.t(),
127127
state
128128
) ::
129-
callback_return
129+
{[Action.common_actions()], state()}
130130

131131
@doc """
132132
Callback invoked when pipeline switches the playback to `:playing`.
@@ -136,7 +136,7 @@ defmodule Membrane.Pipeline do
136136
context :: CallbackContext.t(),
137137
state
138138
) ::
139-
callback_return
139+
{[Action.common_actions()], state()}
140140

141141
@doc """
142142
Callback invoked when a child removes its pad.
@@ -151,7 +151,7 @@ defmodule Membrane.Pipeline do
151151
pad :: Pad.ref(),
152152
context :: CallbackContext.t(),
153153
state :: state
154-
) :: callback_return
154+
) :: {[Action.common_actions()], state()}
155155

156156
@doc """
157157
Callback invoked when a notification comes in from a child.
@@ -163,7 +163,7 @@ defmodule Membrane.Pipeline do
163163
element :: Child.name(),
164164
context :: CallbackContext.t(),
165165
state
166-
) :: callback_return
166+
) :: {[Action.common_actions()], state()}
167167

168168
@doc """
169169
Callback invoked when pipeline receives a message that is not recognized
@@ -177,7 +177,7 @@ defmodule Membrane.Pipeline do
177177
context :: CallbackContext.t(),
178178
state
179179
) ::
180-
callback_return
180+
{[Action.common_actions()], state()}
181181

182182
@doc """
183183
Callback invoked when a child element starts processing stream via given pad.
@@ -189,7 +189,7 @@ defmodule Membrane.Pipeline do
189189
pad :: Pad.ref(),
190190
context :: CallbackContext.t(),
191191
state
192-
) :: callback_return
192+
) :: {[Action.common_actions()], state()}
193193

194194
@doc """
195195
Callback invoked when a child element finishes processing stream via given pad.
@@ -201,7 +201,7 @@ defmodule Membrane.Pipeline do
201201
pad :: Pad.ref(),
202202
context :: CallbackContext.t(),
203203
state
204-
) :: callback_return
204+
) :: {[Action.common_actions()], state()}
205205

206206
@doc """
207207
Callback invoked when children of `Membrane.ChildrenSpec` are started.
@@ -212,7 +212,7 @@ defmodule Membrane.Pipeline do
212212
children :: [Child.name()],
213213
context :: CallbackContext.t(),
214214
state
215-
) :: callback_return
215+
) :: {[Action.common_actions()], state()}
216216

217217
@doc """
218218
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
222222
timer_id :: any,
223223
context :: CallbackContext.t(),
224224
state
225-
) :: callback_return
225+
) :: {[Action.common_actions()], state()}
226226

227227
@doc """
228228
Callback invoked when crash of the crash group happens.
@@ -234,7 +234,7 @@ defmodule Membrane.Pipeline do
234234
group_name :: Child.group(),
235235
context :: CallbackContext.t(),
236236
state
237-
) :: callback_return
237+
) :: {[Action.common_actions()], state()}
238238

239239
@doc """
240240
Callback invoked when pipeline is called using a synchronous call.
@@ -247,7 +247,7 @@ defmodule Membrane.Pipeline do
247247
context :: CallbackContext.t(),
248248
state
249249
) ::
250-
callback_return
250+
{[Action.common_actions() | Action.reply()], state()}
251251

252252
@optional_callbacks handle_init: 2,
253253
handle_setup: 2,

0 commit comments

Comments
 (0)