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

Create Ogg.Muxer #12

Merged
merged 34 commits into from
Jun 6, 2024
Merged

Create Ogg.Muxer #12

merged 34 commits into from
Jun 6, 2024

Conversation

Noarkhh
Copy link
Contributor

@Noarkhh Noarkhh commented May 15, 2024

@Noarkhh Noarkhh changed the title Create OGG.Muxer Create Ogg.Muxer May 20, 2024
@Noarkhh Noarkhh force-pushed the create-muxer branch 2 times, most recently from 828924e to f630876 Compare May 21, 2024 16:11
@Noarkhh Noarkhh marked this pull request as ready for review May 21, 2024 16:31
@Noarkhh Noarkhh requested a review from mat-hek May 21, 2024 16:31
lib/muxer.ex Outdated
alias Membrane.Ogg.Page

def_input_pad :input,
flow_control: :auto,
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
flow_control: :auto,

lib/muxer.ex Outdated
accepted_format: %Membrane.Opus{self_delimiting?: false}

def_output_pad :output,
flow_control: :auto,
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
flow_control: :auto,

lib/muxer.ex Outdated
end

@impl true
def handle_buffer(:input, %Buffer{pts: pts} = buffer, _ctx, state) when not is_nil(pts) do
Copy link
Member

Choose a reason for hiding this comment

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

let's check for the duration in metadata as well

lib/opus.ex Outdated
@spec create_plc_packets(Membrane.Time.t(), Membrane.Time.t()) :: [plc_packet()]
def create_plc_packets(gap_start_timestamp, gap_duration) do
# PLC = Packet Loss Concealment
if rem(gap_duration, @shortest_frame_duration) != 0, do: raise("Unrecoverable gap")
Copy link
Member

Choose a reason for hiding this comment

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

I guess we shouldn't expect PTS to be perfectly precise. I'd add some tolerance factor here and maybe just warn in that case. Please try using this in a WebRTC -> OGG pipeline. You can use this example as a reference: https://github.com/membraneframework/membrane_webrtc_plugin/blob/master/examples/browser_to_file.exs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When running this example the timestamps are precise, but I'll add some error margin just in case

lib/muxer.ex Outdated
encapsulate_packets(
rest_packets,
%{state | total_duration: state.total_duration + first_packet.metadata.duration},
actions ++ new_actions
Copy link
Member

Choose a reason for hiding this comment

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

since it seems we only accumulate buffers here, let's make it buffers instead of actions and send them all in a single action

README.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

let's mention that the examples are in the examples folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was already mentioned, but I noticed the example was really outdated, so I updated it

@Noarkhh Noarkhh requested a review from mat-hek June 4, 2024 11:05
@Noarkhh Noarkhh merged commit 266137e into master Jun 6, 2024
3 checks passed
@Noarkhh Noarkhh deleted the create-muxer branch June 6, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Membrane.OGG.Muxer
2 participants