-
Notifications
You must be signed in to change notification settings - Fork 1
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
Rewrite muxer on TimestampQueue #30
Conversation
049cd8f
to
a8b636b
Compare
lib/membrane_flv_plugin/muxer.ex
Outdated
|
||
@impl true | ||
def handle_init(_ctx, _opts) do | ||
queue = | ||
TimestampQueue.new( | ||
pause_demand_boundary: Membrane.Time.milliseconds(100), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like we should have a default for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default is :infinity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problematic thing is that boundary may be pos_integer()
or Membrane.Time.t()
depending on the pause_demand_boundary_unit
value (:buffers
, :bytes
or :time
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe pause_demand_boundary: {unit, value}
would be better then?
lib/membrane_flv_plugin/muxer.ex
Outdated
# Check if there are any input pads that didn't eos. If not, send end of stream on output | ||
state = Map.update!(state, :last_dts, &Map.delete(&1, pad)) | ||
defp handle_queue_item({pad, :end_of_stream}, state) do | ||
state = Map.update!(state, :inputs_without_eos, &MapSet.delete(&1, pad)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add checking for pads with eos to queue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, but what should be returned for a pad, that has eos stored in the queue, but this eos hasn't been popped from the queue yet? How will this be deduced from the API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, pads are removed from the queue on popping eos, so maybe a function that would return registered pads?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d96551a
to
4fe9553
Compare
mix.exs
Outdated
@@ -40,7 +40,8 @@ defmodule Membrane.FLV.Mixfile do | |||
{:membrane_core, "~> 1.0"}, | |||
{:membrane_aac_format, "~> 0.8.0"}, | |||
{:membrane_h264_format, "~> 0.6.1"}, | |||
{:membrane_timestamp_queue, "~> 0.1.0"}, | |||
{:membrane_timestamp_queue, "~> 0.2.1"}, | |||
# {:membrane_timestamp_queue, path: "../membrane_timestamp_queue"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover
No description provided.