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

Time conversion for rationals #435

Merged
merged 9 commits into from
Jul 11, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Add `handle_call/3` callback in the pipeline, as well as a `:reply` and `:reply_to` actions. Rename `handle_other/3` callback into `handle_info/3` [#334](https://github.com/membraneframework/membrane_core/issues/334)
* Add `Membrane.FilterAggregator` that allows to run multiple filters sequentially within one process. [#355](https://github.com/membraneframework/membrane_core/pull/355)
* Log info about element's playback state change as debug, not as debug_verbose. [#430](https://github.com/membraneframework/membrane_core/pull/430)
* Rename `Membrane.Time.to_<unit name>/1` into `Membrane.Time.round_to_<unit name>/1` to indicate that the result will be rounded. Make `Membrane.Time.<plural unit name>/1` accept `%Ratio{}` as an argument.

## 0.10.0
* Remove all deprecated stuff [#399](https://github.com/membraneframework/membrane_core/pull/399)
Expand Down
2 changes: 1 addition & 1 deletion lib/membrane/core/timer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ defmodule Membrane.Core.Timer do
} = timer

time_passed = time_passed + interval
time = (init_time + time_passed / ratio) |> Ratio.floor() |> Time.to_milliseconds()
time = (init_time + time_passed / ratio) |> Ratio.floor() |> Time.round_to_milliseconds()
timer_ref = Process.send_after(self(), Message.new(:timer_tick, id), time, abs: true)
%__MODULE__{timer | time_passed: time_passed, timer_ref: timer_ref}
end
Expand Down
2 changes: 1 addition & 1 deletion lib/membrane/sync.ex
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ defmodule Membrane.Sync do
|> Enum.filter(&(&1.status == :sync))
|> Enum.group_by(& &1.latency, & &1.reply_to)
|> Enum.each(fn {latency, reply_to} ->
time = (max_latency - latency) |> Time.to_milliseconds()
time = (max_latency - latency) |> Time.round_to_milliseconds()
Process.send_after(self(), {:reply, reply_to}, time)
end)
end
Expand Down
47 changes: 41 additions & 6 deletions lib/membrane/time.ex
Original file line number Diff line number Diff line change
Expand Up @@ -239,21 +239,33 @@ defmodule Membrane.Time do
@doc """
Returns given amount of #{unit.plural} in `#{inspect(__MODULE__)}` units.
"""
@spec unquote(unit.plural)(integer) :: t
@spec unquote(unit.plural)(integer | Ratio.t()) :: t
# credo:disable-for-next-line Credo.Check.Readability.Specs
def unquote(unit.plural)(number) when is_integer(number) do
number * unquote(unit.duration)
end

to_fun_name = :"to_#{unit.plural}"
# credo:disable-for-next-line Credo.Check.Readability.Specs
def unquote(unit.plural)(number) do
if not Ratio.is_rational?(number),
do:
raise(
"Only integers and rationals can be converted with Membrane.Time.#{unquote(unit.plural)}"
)
varsill marked this conversation as resolved.
Show resolved Hide resolved

Ratio.*(number, unquote(unit.duration))
|> round_rational()
end

round_to_fun_name = :"round_to_#{unit.plural}"

@doc """
Returns time in #{unit.plural}. Rounded using `Kernel.round/1`.
Returns time in #{unit.plural}. Rounded to the nearest integer.
"""
@spec unquote(to_fun_name)(t) :: integer
@spec unquote(round_to_fun_name)(t) :: integer
# credo:disable-for-next-line Credo.Check.Readability.Specs
def unquote(to_fun_name)(time) when is_time(time) do
round(time / unquote(unit.duration))
def unquote(round_to_fun_name)(time) when is_time(time) do
Ratio.new(time, unquote(unit.duration)) |> round_rational()
end

as_fun_name = :"as_#{unit.plural}"
Expand All @@ -272,4 +284,27 @@ defmodule Membrane.Time do
unit = @units |> Enum.find(&(rem(time, &1.duration) == 0))
{time |> div(unit.duration), unit}
end

defp round_rational(ratio) do
ratio = make_rational(ratio)
trunced = Kernel.div(ratio.numerator, ratio.denominator)
varsill marked this conversation as resolved.
Show resolved Hide resolved

if 2 * sign_of_rational(ratio) *
Kernel.rem(ratio.numerator, ratio.denominator) >=
ratio.denominator,
do: trunced + sign_of_rational(ratio),
else: trunced
end

defp make_rational(number) do
if Ratio.is_rational?(number) do
number
else
%Ratio{numerator: number, denominator: 1}
end
end

defp sign_of_rational(ratio) do
if ratio.numerator == 0, do: 0, else: Ratio.sign(ratio)
end
end
17 changes: 17 additions & 0 deletions test/membrane/time_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,21 @@ defmodule Membrane.TimeTest do
test "Monotonic time should be integer" do
assert is_integer(@module.monotonic_time())
end

test "Time units functions work properly with rational numbers" do
value = Ratio.new(1, 2)
assert @module.microseconds(value) == 500
assert @module.milliseconds(value) == 500_000
assert @module.seconds(value) == 500_000_000
assert @module.hours(value) == 1_800_000_000_000
assert @module.days(value) == 43_200_000_000_000
end

test "Time units functions properly round the values" do
assert @module.seconds(Ratio.new(1, 3)) == 333_333_333
assert @module.seconds(Ratio.new(-1, 3)) == -333_333_333

assert @module.seconds(Ratio.new(2, 3)) == 666_666_667
assert @module.seconds(Ratio.new(-2, 3)) == -666_666_667
end
end