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

Specify pad codec #114

Merged
merged 11 commits into from
Jul 10, 2024
Merged

Specify pad codec #114

merged 11 commits into from
Jul 10, 2024

Conversation

FelonEkonom
Copy link
Member

No description provided.

@FelonEkonom FelonEkonom requested a review from mat-hek as a code owner June 21, 2024 17:28
@FelonEkonom FelonEkonom marked this pull request as draft June 21, 2024 17:28
@FelonEkonom FelonEkonom marked this pull request as ready for review June 24, 2024 15:13
@FelonEkonom FelonEkonom self-assigned this Jun 24, 2024
@FelonEkonom FelonEkonom added the enhancement New feature or request label Jun 24, 2024
Copy link
Member

@mat-hek mat-hek left a comment

Choose a reason for hiding this comment

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

As we spoke, let's go for kind: :audio | :video pad option

@FelonEkonom FelonEkonom requested a review from mat-hek June 25, 2024 14:13
@@ -24,11 +24,11 @@ defmodule Example do
hackney_opts: [follow_redirect: true]
})
|> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
|> via_out(Pad.ref(:output, 1))
|> via_out(Pad.ref(:output, 1), options: [kind: :video])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
|> via_out(Pad.ref(:output, 1), options: [kind: :video])
|> via_out(:output, options: [kind: :video])

|> child(:parser_video, %Membrane.H264.Parser{output_stream_structure: :annexb})
|> child(:sink_video, %Membrane.File.Sink{location: @output_video}),
get_child(:demuxer)
|> via_out(Pad.ref(:output, 2))
|> via_out(Pad.ref(:output, 2), options: [kind: :audio])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
|> via_out(Pad.ref(:output, 2), options: [kind: :audio])
|> via_out(:output, options: [kind: :audio])

Comment on lines 416 to 483
defp match_tracks_with_pads(ctx, state) do
kind_to_pads_data =
ctx.pads
|> Map.values()
|> Enum.reject(fn %{ref: pad_ref} -> pad_ref == :input end)
|> Enum.group_by(& &1.options.kind)

{track_to_pad_id, empty_kind_to_pads_data} =
state.samples_info.sample_tables
|> Enum.map_reduce(kind_to_pads_data, fn {track_id, table}, kind_to_pads_data ->
track_kind = sample_description_to_kind(table.sample_description)

case kind_to_pads_data do
%{^track_kind => [pad_data | tail]} ->
# match track with a pad with specified kind
kind_to_pads_data = Map.put(kind_to_pads_data, track_kind, tail)
Pad.ref(:output, pad_id) = pad_data.ref
{{track_id, pad_id}, kind_to_pads_data}

%{nil => [pad_data]} ->
# match track with the only one pad with unspecified kind
kind_to_pads_data = Map.delete(kind_to_pads_data, nil)
Pad.ref(:output, pad_id) = pad_data.ref
{{track_id, pad_id}, kind_to_pads_data}

%{} ->
# corresponding pad hasn't been linked yet
{{track_id, nil}, kind_to_pads_data}
end
end)

max_pad_id =
track_to_pad_id
|> Enum.flat_map(fn {_track, pad_id} -> if pad_id != nil, do: [pad_id], else: [] end)
|> Enum.max(&>=/2, fn -> 0 end)

{track_to_pad_id, _max_pad_id} =
track_to_pad_id
|> Enum.map_reduce(max_pad_id, fn
{track_id, nil}, max_pad_id -> {{track_id, max_pad_id + 1}, max_pad_id + 1}
track_pad_pair, max_pad_id -> {track_pad_pair, max_pad_id}
end)

raise? =
empty_kind_to_pads_data
|> Map.values()
|> Enum.any?(&(&1 != []))

if raise? do
pads_kinds =
ctx.pads
|> Enum.flat_map(fn
{:input, _pad_data} -> []
{_pad_ref, %{options: %{kind: kind}}} -> [kind]
end)

tracks_codecs =
state.samples_info.sample_tables
|> Enum.map(fn {_track, table} -> table.sample_description.__struct__ end)

raise """
Pads kinds don't match with tracks codecs. Pads kinds are #{inspect(pads_kinds)}. \
Tracks codecs are #{inspect(tracks_codecs)}
"""
end

%{state | track_to_pad_id: Map.new(track_to_pad_id)}
end
Copy link
Member

Choose a reason for hiding this comment

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

Why is it so long and complex? Let's simplify it

Copy link
Member

Choose a reason for hiding this comment

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

please update the moduledoc

@FelonEkonom FelonEkonom requested a review from mat-hek July 8, 2024 15:18
lib/membrane_mp4/demuxer/isom.ex Outdated Show resolved Hide resolved
@FelonEkonom FelonEkonom merged commit 10d4108 into master Jul 10, 2024
3 checks passed
@FelonEkonom FelonEkonom deleted the specify-pad-codec branch July 10, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants