Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ The package can be installed by adding `membrane_transcoder_plugin` to your list
```elixir
def deps do
[
{:membrane_transcoder_plugin, "~> 0.2.2"}
{:membrane_transcoder_plugin, "~> 0.3.0"}
]
end
```
Expand Down
47 changes: 33 additions & 14 deletions lib/transcoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,35 @@ defmodule Membrane.Transcoder do
and is supposed to return the desired output stream format or its module.
"""
],
force_transcoding?: [
spec: boolean() | (stream_format() -> boolean()),
default: false,
transcoding_policy: [
spec:
:always
| :if_needed
| :never
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks a bit off to me, having a transcoder with transcoding_policy: :never :D Let's try thinking about
other option name, perhaps emphasising somehow that there are "heavy" operations and "light operations" would be helpful here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, my first idea was to add forbid_transcoding? flag next to the force_transcoding?, then I thought that transcoding_policy will sound much better that these 2 combined. Do you propose any specific option name and values?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option could be called something like :heavy_operations_policy, or perhaps we should rename the bin since it not only does transcoding

| (stream_format() -> :always | :if_needed | :never),
default: :if_needed,
description: """
If set to `true`, the input media stream will be decoded and encoded, even
if the input stream format and the output stream format are the same type.
Specifies, when transcoding should be applied.

Can be either:
* a boolean,
* a function that receives the input stream format and returns a boolean.
* an atom: `:always`, `:if_needed` (default) or `:never`,
* a function that receives the input stream format and returns either `:always`,
`:if_needed` or `:never`.

If set to `:always`, the input media stream will be decoded and encoded, even
if the input stream format and the output stream format are the same type.

If set to `:if_needed`, the input media stream will be transcoded only if the input
stream format and the output stream format are different types.
This is the default behavior.

If set to `:never`, the input media stream won't be neither decoded nor encoded.
Changing alignment, encapsulation or stream structure is still possible. This option
is helpful when you want to ensure that #{inspect(__MODULE__)} will not use too much
of resources, e.g. CPU or memory.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mention that transcoder will fail if the output format doesn't match input.


If the transition from the input stream format to the output stream format is not
possible without decoding or encoding the stream, an error will be raised.
"""
],
assumed_input_stream_format: [
Expand Down Expand Up @@ -152,16 +171,16 @@ defmodule Membrane.Transcoder do
|> resolve_output_stream_format()

state =
with %{force_transcoding?: f} when is_function(f) <- state do
%{state | force_transcoding?: f.(format)}
with %{transcoding_policy: f} when is_function(f) <- state do
%{state | transcoding_policy: f.(format)}
end

spec =
get_child(:connector)
|> plug_transcoding(
format,
state.output_stream_format,
state.force_transcoding?
state.transcoding_policy
)
|> get_child(:output_funnel)

Expand Down Expand Up @@ -203,15 +222,15 @@ defmodule Membrane.Transcoder do
end
end

defp plug_transcoding(builder, input_format, output_format, force_transcoding?)
defp plug_transcoding(builder, input_format, output_format, transcoding_policy)
when Audio.is_audio_format(input_format) do
builder
|> Audio.plug_audio_transcoding(input_format, output_format, force_transcoding?)
|> Audio.plug_audio_transcoding(input_format, output_format, transcoding_policy)
end

defp plug_transcoding(builder, input_format, output_format, force_transcoding?)
defp plug_transcoding(builder, input_format, output_format, transcoding_policy)
when Video.is_video_format(input_format) do
builder
|> Video.plug_video_transcoding(input_format, output_format, force_transcoding?)
|> Video.plug_video_transcoding(input_format, output_format, transcoding_policy)
end
end
23 changes: 16 additions & 7 deletions lib/transcoder/audio.ex
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,18 @@ defmodule Membrane.Transcoder.Audio do
audio_stream_format(),
boolean()
) :: ChildrenSpec.builder()
def plug_audio_transcoding(builder, input_format, output_format, force_transcoding?)
def plug_audio_transcoding(builder, input_format, output_format, transcoding_policy)
when is_audio_format(input_format) and is_audio_format(output_format) do
do_plug_audio_transcoding(builder, input_format, output_format, force_transcoding?)
do_plug_audio_transcoding(builder, input_format, output_format, transcoding_policy)
end

defp do_plug_audio_transcoding(
builder,
%format_module{},
%format_module{},
false = _force_transcoding?
) do
transcoding_policy
)
when transcoding_policy in [:if_needed, :never] do
Membrane.Logger.debug("""
This bin will only forward buffers, as the input stream format is the same as the output stream format.
""")
Expand All @@ -74,12 +75,20 @@ defmodule Membrane.Transcoder.Audio do
builder,
%RemoteStream{content_format: Opus},
%Opus{},
false = _force_transcoding?
) do
transcoding_policy
)
when transcoding_policy in [:if_needed, :never] do
builder |> child(:opus_parser, Opus.Parser)
end

defp do_plug_audio_transcoding(builder, input_format, output_format, _force_transcoding?) do
defp do_plug_audio_transcoding(_builder, input_format, output_format, :never) do
raise """
Cannot convert input format #{inspect(input_format)} to output format #{inspect(output_format)} \
with :transcoding_policy option set to :never.
"""
end

defp do_plug_audio_transcoding(builder, input_format, output_format, _transcoding_policy) do
builder
|> maybe_plug_parser(input_format)
|> maybe_plug_decoder(input_format)
Expand Down
34 changes: 17 additions & 17 deletions lib/transcoder/video.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,22 @@ defmodule Membrane.Transcoder.Video do
video_stream_format(),
boolean()
) :: ChildrenSpec.builder()
def plug_video_transcoding(builder, input_format, output_format, force_transcoding?)
def plug_video_transcoding(builder, input_format, output_format, transcoding_policy)
when is_video_format(input_format) and is_video_format(output_format) do
do_plug_video_transcoding(builder, input_format, output_format, force_transcoding?)
do_plug_video_transcoding(builder, input_format, output_format, transcoding_policy)
end

defp do_plug_video_transcoding(
builder,
%H264{},
%H264{} = output_format,
false = _force_transcoding?
) do
defp do_plug_video_transcoding(builder, %H264{}, %H264{} = output_format, transcoding_policy)
when transcoding_policy in [:if_needed, :never] do
builder
|> child(:h264_parser, %H264.Parser{
output_stream_structure: stream_structure_type(output_format),
output_alignment: output_format.alignment
})
end

defp do_plug_video_transcoding(
builder,
%H265{},
%H265{} = output_format,
false = _force_transcoding?
) do
defp do_plug_video_transcoding(builder, %H265{}, %H265{} = output_format, transcoding_policy)
when transcoding_policy in [:if_needed, :never] do
builder
|> child(:h265_parser, %H265.Parser{
output_stream_structure: stream_structure_type(output_format),
Expand All @@ -54,16 +46,24 @@ defmodule Membrane.Transcoder.Video do
builder,
%format_module{},
%format_module{},
false = _force_transcoding?
) do
transcoding_policy
)
when transcoding_policy in [:if_needed, :never] do
Membrane.Logger.debug("""
This bin will only forward buffers, as the input stream format is the same type as the output stream format.
""")

builder
end

defp do_plug_video_transcoding(builder, input_format, output_format, _force_transcoding?) do
defp do_plug_video_transcoding(_builder, input_format, output_format, :never) do
raise """
Cannot convert input format #{inspect(input_format)} to output format #{inspect(output_format)} \
with :transcoding_policy option set to :never.
"""
end

defp do_plug_video_transcoding(builder, input_format, output_format, _transcoding_policy) do
builder
|> maybe_plug_parser_and_decoder(input_format)
|> maybe_plug_encoder_and_parser(output_format)
Expand Down
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
defmodule Membrane.Transcoder.Plugin.Mixfile do
use Mix.Project

@version "0.2.2"
@version "0.3.0"
@github_url "https://github.com/membraneframework/membrane_transcoder_plugin"

def project do
Expand Down
37 changes: 32 additions & 5 deletions test/integration_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,20 @@ defmodule Membrane.Transcoder.IntegrationTest do
do: {[stream_format: {:output, state.format}], state}
end

test "if encoder and decoder are spawned or not, depending on the value of `force_transcoding?` option" do
test "if encoder and decoder are spawned or not, depending on the value of `transcoding_policy` option" do
for format <- [%AAC{channels: 1}, %H264{alignment: :au, stream_structure: :annexb}],
force_transcoding? <- [true, false] do
transcoding_policy <- [:always, :if_needed, :never] do
output_format =
case format do
%H264{} -> %H264{format | stream_structure: :avc1}
%AAC{} -> format
end

spec =
child(:source, %FormatSource{format: format})
|> child(:transcoder, %Membrane.Transcoder{
output_stream_format: format,
force_transcoding?: force_transcoding?
output_stream_format: output_format,
transcoding_policy: transcoding_policy
})
|> child(:sink, Testing.Sink)

Expand All @@ -98,7 +104,7 @@ defmodule Membrane.Transcoder.IntegrationTest do
|> Enum.each(fn child_name ->
get_child_result = Testing.Pipeline.get_child_pid(pipeline, [:transcoder, child_name])

if force_transcoding? do
if transcoding_policy == :always do
assert {:ok, child_pid} = get_child_result
assert is_pid(child_pid)
else
Expand All @@ -109,4 +115,25 @@ defmodule Membrane.Transcoder.IntegrationTest do
Testing.Pipeline.terminate(pipeline)
end
end

test "if transcoder raises when `transcoding_policy` is set to `:never` and formats don't match" do
spec =
child(:source, %FormatSource{format: %H264{alignment: :au, stream_structure: :annexb}})
|> child(:transcoder, %Membrane.Transcoder{
output_stream_format: VP8,
transcoding_policy: :never
})
|> child(:sink, Testing.Sink)

{:ok, supervisor, pipeline} = Testing.Pipeline.start(spec: [])
supervisor_ref = Process.monitor(supervisor)
pipeline_ref = Process.monitor(pipeline)

Testing.Pipeline.execute_actions(pipeline, spec: spec)

assert_receive {:DOWN, ^pipeline_ref, :process, _pid,
{:membrane_child_crash, :transcoder, {%RuntimeError{}, _stacktrace}}}

assert_receive {:DOWN, ^supervisor_ref, :process, _pid, _reason}
end
end