From 14c7422a5b02b35b8b59bbaa5a4743aa382f9aad Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Thu, 20 Feb 2025 17:09:20 +0100 Subject: [PATCH 1/6] Add enforce_encoding? option --- lib/transcoder.ex | 44 +++++++++++++++++++++++----- lib/transcoder/audio.ex | 23 +++++++++++---- lib/transcoder/video.ex | 23 +++++++++++---- test/integration_test.exs | 61 +++++++++++++++++++++++++++++++++++---- 4 files changed, 126 insertions(+), 25 deletions(-) diff --git a/lib/transcoder.ex b/lib/transcoder.ex index 4c25934..3808374 100644 --- a/lib/transcoder.ex +++ b/lib/transcoder.ex @@ -43,10 +43,16 @@ defmodule Membrane.Transcoder do """ @type stream_format_module :: H264 | H265 | VP8 | RawVideo | AAC | Opus | RawAudio + @type transcoding_options :: [enforce_transcoding?: boolean()] + @typedoc """ Describes a function which can be used to provide output format based on the input format. """ - @type stream_format_resolver :: (stream_format() -> stream_format() | stream_format_module()) + @type stream_format_resolver :: (stream_format() -> + stream_format() + | {stream_format(), transcoding_options()} + | stream_format_module() + | {stream_format_module(), transcoding_options()}) def_input_pad :input, accepted_format: format when Audio.is_audio_format(format) or Video.is_video_format(format) @@ -55,7 +61,12 @@ defmodule Membrane.Transcoder do accepted_format: format when Audio.is_audio_format(format) or Video.is_video_format(format) def_options output_stream_format: [ - spec: stream_format() | stream_format_module() | stream_format_resolver(), + spec: + stream_format() + | {stream_format(), transcoding_options()} + | stream_format_module() + | {stream_format_module(), transcoding_options()} + | stream_format_resolver(), description: """ An option specifying desired output format. Can be either: @@ -78,7 +89,10 @@ defmodule Membrane.Transcoder do state = opts |> Map.from_struct() - |> Map.put(:input_stream_format, nil) + |> Map.merge(%{ + input_stream_format: nil, + enforce_transcoding?: false + }) {[spec: spec], state} end @@ -96,7 +110,11 @@ defmodule Membrane.Transcoder do spec = get_child(:forwarding_filter) - |> plug_transcoding(format, state.output_stream_format) + |> plug_transcoding( + format, + state.output_stream_format, + state.enforce_transcoding? + ) |> get_child(:output_funnel) {[spec: spec], state} @@ -127,6 +145,14 @@ defmodule Membrane.Transcoder do defp resolve_output_stream_format(state) do case state.output_stream_format do + {stream_format, options} -> + [enforce_transcoding?: enforce_transcoding?] = + options + |> Keyword.validate!(enforce_transcoding?: false) + + %{state | enforce_transcoding?: enforce_transcoding?, output_stream_format: stream_format} + |> resolve_output_stream_format() + format when is_struct(format) -> state @@ -139,13 +165,15 @@ defmodule Membrane.Transcoder do end end - defp plug_transcoding(builder, input_format, output_format) + defp plug_transcoding(builder, input_format, output_format, enforce_transcoding?) when Audio.is_audio_format(input_format) do - builder |> Audio.plug_audio_transcoding(input_format, output_format) + builder + |> Audio.plug_audio_transcoding(input_format, output_format, enforce_transcoding?) end - defp plug_transcoding(builder, input_format, output_format) + defp plug_transcoding(builder, input_format, output_format, enforce_transcoding?) when Video.is_video_format(input_format) do - builder |> Video.plug_video_transcoding(input_format, output_format) + builder + |> Video.plug_video_transcoding(input_format, output_format, enforce_transcoding?) end end diff --git a/lib/transcoder/audio.ex b/lib/transcoder/audio.ex index 2c71c5e..5f43402 100644 --- a/lib/transcoder/audio.ex +++ b/lib/transcoder/audio.ex @@ -32,14 +32,20 @@ defmodule Membrane.Transcoder.Audio do @spec plug_audio_transcoding( ChildrenSpec.builder(), audio_stream_format() | RemoteStream.t(), - audio_stream_format() + audio_stream_format(), + boolean() ) :: ChildrenSpec.builder() - def plug_audio_transcoding(builder, input_format, output_format) + def plug_audio_transcoding(builder, input_format, output_format, enforce_transcoding?) when is_audio_format(input_format) and is_audio_format(output_format) do - do_plug_audio_transcoding(builder, input_format, output_format) + do_plug_audio_transcoding(builder, input_format, output_format, enforce_transcoding?) end - defp do_plug_audio_transcoding(builder, %format_module{}, %format_module{}) do + defp do_plug_audio_transcoding( + builder, + %format_module{}, + %format_module{}, + false = _enforce_transcoding? + ) do Membrane.Logger.debug(""" This bin will only forward buffers, as the input stream format is the same as the output stream format. """) @@ -47,11 +53,16 @@ defmodule Membrane.Transcoder.Audio do builder end - defp do_plug_audio_transcoding(builder, %RemoteStream{content_format: Opus}, %Opus{}) do + defp do_plug_audio_transcoding( + builder, + %RemoteStream{content_format: Opus}, + %Opus{}, + false = _enforce_transcoding? + ) do builder |> child(:opus_parser, Opus.Parser) end - defp do_plug_audio_transcoding(builder, input_format, output_format) do + defp do_plug_audio_transcoding(builder, input_format, output_format, _enforce_transcoding?) do builder |> maybe_plug_parser(input_format) |> maybe_plug_decoder(input_format) diff --git a/lib/transcoder/video.ex b/lib/transcoder/video.ex index b025ebd..3aed1b3 100644 --- a/lib/transcoder/video.ex +++ b/lib/transcoder/video.ex @@ -16,14 +16,20 @@ defmodule Membrane.Transcoder.Video do @spec plug_video_transcoding( ChildrenSpec.builder(), video_stream_format(), - video_stream_format() + video_stream_format(), + boolean() ) :: ChildrenSpec.builder() - def plug_video_transcoding(builder, input_format, output_format) + def plug_video_transcoding(builder, input_format, output_format, enforce_transcoding?) when is_video_format(input_format) and is_video_format(output_format) do - do_plug_video_transcoding(builder, input_format, output_format) + do_plug_video_transcoding(builder, input_format, output_format, enforce_transcoding?) end - defp do_plug_video_transcoding(builder, %h26x{}, %h26x{} = output_format) + defp do_plug_video_transcoding( + builder, + %h26x{}, + %h26x{} = output_format, + false = _enforce_transcoding? + ) when h26x in [H264, H265] do parser = h26x @@ -36,7 +42,12 @@ defmodule Membrane.Transcoder.Video do builder |> child(:h264_parser, parser) end - defp do_plug_video_transcoding(builder, %format_module{}, %format_module{}) do + defp do_plug_video_transcoding( + builder, + %format_module{}, + %format_module{}, + false = _enforce_transcoding? + ) do Membrane.Logger.debug(""" This bin will only forward buffers, as the input stream format is the same type as the output stream format. """) @@ -44,7 +55,7 @@ defmodule Membrane.Transcoder.Video do builder end - defp do_plug_video_transcoding(builder, input_format, output_format) do + defp do_plug_video_transcoding(builder, input_format, output_format, _enforce_transcoding?) do builder |> maybe_plug_parser_and_decoder(input_format) |> maybe_plug_encoder_and_parser(output_format) diff --git a/test/integration_test.exs b/test/integration_test.exs index 6f0a3bf..83e3e97 100644 --- a/test/integration_test.exs +++ b/test/integration_test.exs @@ -4,7 +4,7 @@ defmodule Membrane.Transcoder.IntegrationTest do import Membrane.ChildrenSpec alias Membrane.{AAC, H264, H265, Opus, RawAudio, RawVideo, VP8} - alias Membrane.Testing.Pipeline + alias Membrane.Testing alias Membrane.Transcoder.Support.Preprocessors @video_inputs [ @@ -36,7 +36,7 @@ defmodule Membrane.Transcoder.IntegrationTest do Enum.map(@test_cases, fn test_case -> test "if transcoder support #{inspect(test_case.input_format)} input and #{inspect(test_case.output_format)} output" do - pid = Pipeline.start_link_supervised!() + pid = Testing.Pipeline.start_link_supervised!() spec = child(%Membrane.File.Source{ @@ -44,12 +44,63 @@ defmodule Membrane.Transcoder.IntegrationTest do }) |> then(unquote(test_case.preprocess)) |> child(%Membrane.Transcoder{output_stream_format: unquote(test_case.output_format)}) - |> child(:sink, Membrane.Testing.Sink) + |> child(:sink, Testing.Sink) - Pipeline.execute_actions(pid, spec: spec) + Testing.Pipeline.execute_actions(pid, spec: spec) assert_sink_stream_format(pid, :sink, %unquote(test_case.output_format){}) - Pipeline.terminate(pid) + Testing.Pipeline.terminate(pid) end end) + + defmodule FormatSource do + use Membrane.Source + + def_output_pad :output, accepted_format: _any, flow_control: :push + def_options format: [] + + @impl true + def handle_init(_ctx, opts), do: {[], opts |> Map.from_struct()} + + @impl true + def handle_playing(_ctx, state), + do: {[stream_format: {:output, state.format}], state} + end + + @tag :a + test "a" do + for format <- [ + %Membrane.AAC{channels: 1}, + %Membrane.H264{alignment: :au, stream_structure: :annexb} + ], + enforce_transcoding? <- [true, false] do + spec = + child(:source, %FormatSource{format: format}) + |> child(:transcoder, %Membrane.Transcoder{ + output_stream_format: {format, enforce_transcoding?: enforce_transcoding?} + }) + |> child(:sink, Testing.Sink) + + pipeline = Testing.Pipeline.start_link_supervised!(spec: spec) + + Process.sleep(500) + + case format do + %H264{} -> [:h264_encoder, :h264_decoder] + %AAC{} -> [:aac_encoder, :aac_decoder] + end + |> Enum.each(fn child_name -> + get_child_result = Testing.Pipeline.get_child_pid(pipeline, [:transcoder, child_name]) + + if enforce_transcoding? do + assert {:ok, child_pid} = get_child_result + assert is_pid(child_pid) + else + assert {:error, :child_not_found} = get_child_result + end + end) + + Testing.Pipeline.terminate(pipeline) + end + end end From 07fe7e43342c377ab3750702359a7c6345adebe2 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Thu, 20 Feb 2025 17:31:30 +0100 Subject: [PATCH 2/6] Move enforce_transcoding? to separate option --- lib/transcoder.ex | 39 ++++++++++++++++++++------------------- test/integration_test.exs | 3 ++- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/transcoder.ex b/lib/transcoder.ex index 3808374..62b44ce 100644 --- a/lib/transcoder.ex +++ b/lib/transcoder.ex @@ -43,16 +43,10 @@ defmodule Membrane.Transcoder do """ @type stream_format_module :: H264 | H265 | VP8 | RawVideo | AAC | Opus | RawAudio - @type transcoding_options :: [enforce_transcoding?: boolean()] - @typedoc """ Describes a function which can be used to provide output format based on the input format. """ - @type stream_format_resolver :: (stream_format() -> - stream_format() - | {stream_format(), transcoding_options()} - | stream_format_module() - | {stream_format_module(), transcoding_options()}) + @type stream_format_resolver :: (stream_format() -> stream_format() | stream_format_module()) def_input_pad :input, accepted_format: format when Audio.is_audio_format(format) or Video.is_video_format(format) @@ -63,18 +57,29 @@ defmodule Membrane.Transcoder do def_options output_stream_format: [ spec: stream_format() - | {stream_format(), transcoding_options()} | stream_format_module() - | {stream_format_module(), transcoding_options()} | stream_format_resolver(), description: """ An option specifying desired output format. + Can be either: * a struct being a Membrane stream format, * a module in which Membrane stream format struct is defined, * a function which receives input stream format as an input argument and is supposed to return the desired output stream format or its module. """ + ], + enforce_transcoding?: [ + spec: boolean() | (stream_format() -> boolean()), + default: false, + 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. + + Cen be either: + * a boolean, + * a function that receives the input stream format and returns a boolean. + """ ] @impl true @@ -90,8 +95,7 @@ defmodule Membrane.Transcoder do opts |> Map.from_struct() |> Map.merge(%{ - input_stream_format: nil, - enforce_transcoding?: false + input_stream_format: nil }) {[spec: spec], state} @@ -108,6 +112,11 @@ defmodule Membrane.Transcoder do %{state | input_stream_format: format} |> resolve_output_stream_format() + state = + with %{enforce_transcoding?: f} when is_function(f) <- state do + %{state | enforce_transcoding?: f.(format)} + end + spec = get_child(:forwarding_filter) |> plug_transcoding( @@ -145,14 +154,6 @@ defmodule Membrane.Transcoder do defp resolve_output_stream_format(state) do case state.output_stream_format do - {stream_format, options} -> - [enforce_transcoding?: enforce_transcoding?] = - options - |> Keyword.validate!(enforce_transcoding?: false) - - %{state | enforce_transcoding?: enforce_transcoding?, output_stream_format: stream_format} - |> resolve_output_stream_format() - format when is_struct(format) -> state diff --git a/test/integration_test.exs b/test/integration_test.exs index 83e3e97..8bdf6ca 100644 --- a/test/integration_test.exs +++ b/test/integration_test.exs @@ -77,7 +77,8 @@ defmodule Membrane.Transcoder.IntegrationTest do spec = child(:source, %FormatSource{format: format}) |> child(:transcoder, %Membrane.Transcoder{ - output_stream_format: {format, enforce_transcoding?: enforce_transcoding?} + output_stream_format: format, + enforce_transcoding?: enforce_transcoding? }) |> child(:sink, Testing.Sink) From 7227b7954339093f9787a486f803bb2f3b0fab41 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Thu, 20 Feb 2025 17:36:37 +0100 Subject: [PATCH 3/6] Bump version --- README.md | 2 +- mix.exs | 2 +- test/integration_test.exs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 12a4036..a43054c 100644 --- a/README.md +++ b/README.md @@ -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.1.2"} + {:membrane_transcoder_plugin, "~> 0.1.3"} ] end ``` diff --git a/mix.exs b/mix.exs index 4583f5e..da2cb83 100644 --- a/mix.exs +++ b/mix.exs @@ -1,7 +1,7 @@ defmodule Membrane.Transcoder.Plugin.Mixfile do use Mix.Project - @version "0.1.2" + @version "0.1.3" @github_url "https://github.com/membraneframework/membrane_transcoder_plugin" def project do diff --git a/test/integration_test.exs b/test/integration_test.exs index 8bdf6ca..03370d2 100644 --- a/test/integration_test.exs +++ b/test/integration_test.exs @@ -67,7 +67,6 @@ defmodule Membrane.Transcoder.IntegrationTest do do: {[stream_format: {:output, state.format}], state} end - @tag :a test "a" do for format <- [ %Membrane.AAC{channels: 1}, From 328f97799626b6c87309962a6f7bbfc3dbbe0f50 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Thu, 20 Feb 2025 17:45:13 +0100 Subject: [PATCH 4/6] Fix test name --- test/integration_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration_test.exs b/test/integration_test.exs index 03370d2..d21c4e9 100644 --- a/test/integration_test.exs +++ b/test/integration_test.exs @@ -67,7 +67,7 @@ defmodule Membrane.Transcoder.IntegrationTest do do: {[stream_format: {:output, state.format}], state} end - test "a" do + test "if encoder and decoder are spawned or not, depending on the value of `enforce_transcoding?` option" do for format <- [ %Membrane.AAC{channels: 1}, %Membrane.H264{alignment: :au, stream_structure: :annexb} From 9a6fbbb2318b5ac4c2ecbd77c5845d3e0ca62531 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Thu, 20 Feb 2025 17:48:41 +0100 Subject: [PATCH 5/6] Refactor test --- test/integration_test.exs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/integration_test.exs b/test/integration_test.exs index d21c4e9..9e4c896 100644 --- a/test/integration_test.exs +++ b/test/integration_test.exs @@ -68,10 +68,7 @@ defmodule Membrane.Transcoder.IntegrationTest do end test "if encoder and decoder are spawned or not, depending on the value of `enforce_transcoding?` option" do - for format <- [ - %Membrane.AAC{channels: 1}, - %Membrane.H264{alignment: :au, stream_structure: :annexb} - ], + for format <- [%AAC{channels: 1}, %H264{alignment: :au, stream_structure: :annexb}], enforce_transcoding? <- [true, false] do spec = child(:source, %FormatSource{format: format}) From e9af5aad4f54af1e4241e253bc5b17631b527119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Feliks=20Pobiedzi=C5=84ski?= <38541925+FelonEkonom@users.noreply.github.com> Date: Tue, 25 Feb 2025 11:51:52 +0100 Subject: [PATCH 6/6] Update lib/transcoder.ex Co-authored-by: Mateusz Front --- lib/transcoder.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/transcoder.ex b/lib/transcoder.ex index 62b44ce..b889133 100644 --- a/lib/transcoder.ex +++ b/lib/transcoder.ex @@ -76,7 +76,7 @@ defmodule Membrane.Transcoder do 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. - Cen be either: + Can be either: * a boolean, * a function that receives the input stream format and returns a boolean. """