Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support AVC3 #113

Merged
merged 4 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 lib/membrane_mp4/demuxer/isom.ex
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ defmodule Membrane.MP4.Demuxer.ISOM do
any_of(
%Membrane.AAC{config: {:esds, _esds}},
%Membrane.H264{
stream_structure: {:avc1, _dcr},
stream_structure: {_avc, _dcr},
Copy link
Contributor

Choose a reason for hiding this comment

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

We should assert on _avc being either :avc1 or :avc3

alignment: :au
},
%Membrane.H265{
Expand Down
21 changes: 12 additions & 9 deletions lib/membrane_mp4/muxer/isom.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ defmodule Membrane.MP4.Muxer.ISOM do
any_of(
%Membrane.AAC{config: {:esds, _esds}},
%Membrane.H264{
stream_structure: {:avc1, _dcr},
stream_structure: {_avc, _dcr},
alignment: :au
},
%Membrane.H265{
Expand Down Expand Up @@ -96,25 +96,28 @@ defmodule Membrane.MP4.Muxer.ISOM do
@impl true
def handle_stream_format(
Pad.ref(:input, pad_ref) = pad,
stream_format,
%type{} = stream_format,
ctx,
state
) do
cond do
case ctx.pads[pad].stream_format do
# Handle receiving the first stream format on the given pad
is_nil(ctx.pads[pad].stream_format) ->
nil ->
update_in(state, [:pad_to_track, pad_ref], fn track_id ->
Track.new(track_id, stream_format, state.chunk_duration)
end)

# Handle receiving all but the first stream format on the given pad,
# when stream format is duplicated - ignore
ctx.pads[pad].stream_format == stream_format ->
# Handle receiving a stream format of the same type
%^type{} ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it OK for us to accept change from :avc3 to :avc1? Wouldn't it require appending something to the DCR?

state

# otherwise we can assume that output will be corrupted
true ->
raise "ISOM Muxer doesn't support variable parameters"
previos_format ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
previos_format ->
previous_format ->

raise """
Unsupported stream_format change on pad #{inspect(pad_ref)}, \
previous format: #{inspect(previos_format)}
new format: #{inspect(stream_format)}
"""
end
|> then(&{[], &1})
end
Expand Down
1 change: 0 additions & 1 deletion mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
"mime": {:hex, :mime, "2.0.6", "8f18486773d9b15f95f4f4f1e39b710045fa1de891fada4516559967276e4dc2", [:mix], [], "hexpm", "c9945363a6b26d747389aac3643f8e0e09d30499a138ad64fe8fd1d13d9b153e"},
"mint": {:hex, :mint, "1.6.2", "af6d97a4051eee4f05b5500671d47c3a67dac7386045d87a904126fd4bbcea2e", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0 or ~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "5ee441dffc1892f1ae59127f74afe8fd82fda6587794278d924e4d90ea3d63f9"},
"nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
"nimble_ownership": {:hex, :nimble_ownership, "0.2.1", "3e44c72ebe8dd213db4e13aff4090aaa331d158e72ce1891d02e0ffb05a1eb2d", [:mix], [], "hexpm", "bf38d2ef4fb990521a4ecf112843063c1f58a5c602484af4c7977324042badee"},
"nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
"nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},
"numbers": {:hex, :numbers, "5.2.4", "f123d5bb7f6acc366f8f445e10a32bd403c8469bdbce8ce049e1f0972b607080", [:mix], [{:coerce, "~> 1.0", [hex: :coerce, repo: "hexpm", optional: false]}, {:decimal, "~> 1.9 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "eeccf5c61d5f4922198395bf87a465b6f980b8b862dd22d28198c5e6fab38582"},
Expand Down
Binary file added test/fixtures/isom/ref_video_vp.mp4
Binary file not shown.
72 changes: 18 additions & 54 deletions test/membrane_mp4/muxer/isom/integration_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,24 @@ defmodule Membrane.MP4.Muxer.ISOM.IntegrationTest do
perform_test(pid, "video")
end

test "variable parameters H264 track" do
prepare_test("video")

structure = [
child(:file, %Membrane.File.Source{location: "test/fixtures/in_video_vp.h264"})
|> child(:parser, %Membrane.H264.Parser{
generate_best_effort_timestamps: %{framerate: {30, 1}},
output_stream_structure: :avc3
})
|> child(:muxer, %Membrane.MP4.Muxer.ISOM{chunk_duration: Time.seconds(1)})
|> child(:sink, %Membrane.File.Sink{location: out_path_for("video_vp")})
]

pid = Pipeline.start_link_supervised!(spec: structure)

perform_test(pid, "video_vp")
end

test "single H265 track" do
prepare_test("video_hevc")

Expand Down Expand Up @@ -210,58 +228,4 @@ defmodule Membrane.MP4.Muxer.ISOM.IntegrationTest do
perform_test(pid, "two_tracks_fast_start")
end
end

describe "When fed a variable parameter h264 stream, Muxer.ISOM should" do
test "raise when stream format's inband_parameters are not used" do
structure = [
child(:file, %Membrane.File.Source{location: "test/fixtures/in_video_vp.h264"})
|> child(:parser, %Membrane.H264.Parser{
generate_best_effort_timestamps: %{framerate: {30, 1}},
output_stream_structure: :avc1
})
|> child(:muxer, %Membrane.MP4.Muxer.ISOM{
chunk_duration: Time.seconds(1),
fast_start: true
})
|> child(:sink, Membrane.Fake.Sink.Buffers)
]

{:ok, _supervisor_pid, pid} = Pipeline.start(spec: structure)
monitor_ref = Process.monitor(pid)

assert_receive {:DOWN, ^monitor_ref, :process, ^pid,
{:membrane_child_crash, :muxer,
{%RuntimeError{
message: "ISOM Muxer doesn't support variable parameters"
}, _stacktrace}}},
1_000
end
end

describe "ctts table" do
varsill marked this conversation as resolved.
Show resolved Hide resolved
test "should not be stored when dts and pts values are equal" do
prepare_test("video")

structure = [
child(:file, %Membrane.File.Source{location: "test/fixtures/in_video.h264"})
|> child(:parser, %Membrane.H264.Parser{
generate_best_effort_timestamps: %{framerate: {30, 1}, add_dts_offset: false},
output_stream_structure: :avc1
})
|> child(:muxer, %Membrane.MP4.Muxer.ISOM{chunk_duration: Time.seconds(1)})
|> child(:sink, %Membrane.File.Sink{location: out_path_for("video")})
]

pid = Pipeline.start_link_supervised!(spec: structure)

assert_end_of_stream(pid, :sink, :input)
refute_sink_buffer(pid, :sink, _buffer, 0)

assert :ok == Pipeline.terminate(pid)

assert {parsed_out, <<>>} = out_path_for("video") |> File.read!() |> Container.parse!()
assert Container.get_box(parsed_out, [:moov, :trak, :mdia, :minf, :stbl, :stts])
refute Container.get_box(parsed_out, [:moov, :trak, :mdia, :minf, :stbl, :ctts])
end
end
end