Add option to disable realtimer for outputs #98
Conversation
lib/boombox/internal_bin/rtp.ex
Outdated
| }, | ||
| transcoding_policy: :always | :if_needed | :never | ||
| transcoding_policy: :always | :if_needed | :never, | ||
| ignore_timestamps: boolean() |
There was a problem hiding this comment.
trying to do a small brainstorm here: did you have any other ideas about what could be the name of this option?
There was a problem hiding this comment.
Yup, we definitely need a better name. This sounds like we somehow discard timestamps, while we just don't apply pacing. So I'd call it pace_control or realtime or similar. Also, a typedoc would be useful.
lib/boombox.ex
Outdated
| @type out_webrtc_opts :: [ | ||
| transcoding_policy_opt() | ||
| | pace_control_opt() | ||
| ] |
There was a problem hiding this comment.
I'm not sure it should be a separate type. It's not complex, and it's more convenient to have everything in one place when reading the docs
lib/boombox.ex
Outdated
| | pace_control_opt() | ||
| ] | ||
|
|
||
| @type common_input :: |
There was a problem hiding this comment.
How about having just Boombox.input instead of common_input and typing Boombox.run as Boombox.run(Boombox.input() | Boombox.stream_input(), ..., same for the output?
| @typedoc """ | ||
| Determines whether the incoming streams should be passed to the output according to their | ||
| timestamps or as fast as possible. | ||
| """ | ||
| @type pace_control_opt :: {:pace_control, boolean()} |
There was a problem hiding this comment.
We should mention what the default is
Noarkhh
force-pushed
the
add-realtime-option
branch
from
6268207 to
c126e10
Compare
|
@Noarkhh As we spoke, there's a slight shift in our approach :D Let's add the pace control option only to the |
lib/boombox.ex
Outdated
| @type in_stream_opts :: [ | ||
| {:audio, :binary | boolean()} | ||
| | {:video, :image | boolean()} | ||
| | {:realtime, boolean()} |
There was a problem hiding this comment.
maybe is_realtime or is_live 🤔 cc @varsill
otherwise LGTM
| @type in_stream_opts :: [ | ||
| {:audio, :binary | boolean()} | ||
| | {:video, :image | boolean()} | ||
| | {:is_live, boolean()} |
There was a problem hiding this comment.
What is the reaon of having this, if we have pace_control_opt for output?
There was a problem hiding this comment.
We need to know whether the incoming stream is real-time or not, same as :vod | :live in HLS.
FelonEkonom
requested review from
FelonEkonom
and removed request for
FelonEkonom
Some problems can arise when a realtime output, like RTP and WebRTC, is used in conjunction with realtime input - the Realtimer that is in the pipeline can introduce unwanted latency. This PR introduces an option to disable the Realtimers in these circumstances.
The option name is up to debate, this one is most likely a placeholder.