Rework audio pipelines assembly to match the reworked video pipelines assembly - #27
Conversation
be3857a to
203b189
Compare
2273ee4 to
a5bedd6
Compare
…/membrane_transcoder_plugin into rework-audio-pipelines
|
|
||
| case {input_format, output_format} do | ||
| {input_format, %OutputFormat.AAC{}} when is_aac(input_format) -> | ||
| builder |> child({:aac_input_parser, suffix}, Membrane.AAC.Parser) |
There was a problem hiding this comment.
Aren't we silently ignoring options like output_config from the output_spec here?
| delimitation: if(output_format.self_delimiting?, do: :delimit, else: :undelimit) | ||
| }) | ||
|
|
||
| _other -> |
There was a problem hiding this comment.
Won't it "pass" %RemoteStream{content: MPEGAudio} without changing anything?
| (input_format.__struct__ == Membrane.RawAudio and | ||
| output_format.__struct__ == OutputFormat.RawAudio) |
There was a problem hiding this comment.
Why would be always transcode raw_audio -> raw_audio?
| Transcoder.transcoding_policy(), | ||
| Transcoder.State.OutputSpec.t() | ||
| ) :: ChildrenSpec.builder() | ||
| def plug_audio_transcoding( |
There was a problem hiding this comment.
Ugh, I don't like that name - I would call it sth like "convert" since we decide whether or not to transcode :D
(I think it also applies to the video branch and plug_video_transcoding)
|
|
||
| channels = | ||
| @spec are_same_formats(input_format(), output_format()) :: boolean() | ||
| defp are_same_formats(input_format, output_format) do |
There was a problem hiding this comment.
I think we could move it to a common place (OutputFormat perhaps?) since it's exactly the same thing as Video.are_same_formats/2
…Stream{content: MPEGAudio} -> MPEGAudio since there is no parser, don't transcode raw audio if fields do not change, make sure AAC Parser options are not ignored, move is_same_format to common OutputFormat module, rename plug_transcoding into plug_conversion as it does not always perform transcoding
|
I've added my suggested changes and bumped version to v0.5.0 in a9a7e93 |
FelonEkonom
left a comment
FelonEkonom
left a comment
There was a problem hiding this comment.
This PR should update examples
3 participants
No description provided.