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

Bump Ratio to 3.0 #438

Merged
merged 7 commits into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/membrane/clock.ex
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@ defmodule Membrane.Clock do
end

defp handle_clock_update(till_next, state) do
use Ratio
use Numbers, overload_operators: true

if till_next < 0 do
if Ratio.lt?(till_next, 0) do
Copy link
Member

Choose a reason for hiding this comment

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

why do we overload operators if we don't use them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, we are using them - the overloaded operators are not only comparison operators, but also arithmetic operators

raise "Clock update time cannot be negative, received: #{inspect(till_next)}"
end

Expand All @@ -227,10 +227,10 @@ defmodule Membrane.Clock do
end

defp do_handle_clock_update(till_next, state) do
use Ratio
use Numbers, overload_operators: true
%{till_next: from_previous, clock_time: clock_time} = state
clock_time = clock_time + from_previous
ratio = clock_time / (state.time_provider.() - state.init_time)
ratio = Ratio.div(Ratio.new(clock_time), Ratio.new(state.time_provider.() - state.init_time))
Copy link
Member

Choose a reason for hiding this comment

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

as above

state = %{state | clock_time: clock_time, till_next: till_next}
broadcast_and_update_ratio(ratio, state)
end
Expand Down
6 changes: 4 additions & 2 deletions lib/membrane/core/timer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ defmodule Membrane.Core.Timer do
end

def tick(timer) do
use Ratio
use Numbers, overload_operators: true
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 be consistent and not use that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


%__MODULE__{
id: id,
Expand All @@ -62,7 +62,9 @@ defmodule Membrane.Core.Timer do

# Next tick time converted to BEAM clock time
beam_next_tick_time =
(init_time + next_tick_time / ratio) |> Ratio.floor() |> Time.round_to_milliseconds()
(init_time + Ratio.new(next_tick_time, ratio))
|> Ratio.floor()
|> Time.round_to_milliseconds()

timer_ref =
Process.send_after(self(), Message.new(:timer_tick, id), beam_next_tick_time, abs: true)
Expand Down
23 changes: 7 additions & 16 deletions lib/membrane/time.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ defmodule Membrane.Time do
that do not touch hardware clock, you should use Membrane units for consistency.
"""

require Ratio

@compile {:inline,
native_units: 1, native_unit: 0, nanoseconds: 1, nanosecond: 0, second: 0, seconds: 1}

Expand All @@ -30,7 +32,7 @@ defmodule Membrane.Time do
# Difference between 01.01.1900 (start of NTP epoch) and 01.01.1970 (start of Unix epoch) in seconds
@ntp_unix_epoch_diff 2_208_988_800

@two_to_pow_32 Ratio.pow(2, 32)
@two_to_pow_32 Ratio.pow(Ratio.new(2, 1), 32) |> Ratio.trunc()

@doc """
Checks whether a value is `Membrane.Time.t`.
Expand Down Expand Up @@ -260,12 +262,10 @@ defmodule Membrane.Time do
end

# 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)}"
end
def unquote(unit.plural)(number) when Ratio.is_rational(number) do
use Numbers, overload_operators: true
Copy link
Member

Choose a reason for hiding this comment

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

as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done here and also in clock_test.exs


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

Expand All @@ -288,7 +288,7 @@ defmodule Membrane.Time do
@spec unquote(as_fun_name)(t) :: integer | Ratio.t()
# credo:disable-for-next-line Credo.Check.Readability.Specs
def unquote(as_fun_name)(time) when is_time(time) do
Ratio./(time, unquote(unit.duration))
Ratio.new(time, unquote(unit.duration))
end
end)

Expand All @@ -298,7 +298,6 @@ defmodule Membrane.Time do
end

defp round_rational(ratio) do
ratio = make_rational(ratio)
trunced = Ratio.trunc(ratio)

if 2 * sign_of_rational(ratio) *
Expand All @@ -308,14 +307,6 @@ defmodule Membrane.Time do
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
Expand Down
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ defmodule Membrane.Mixfile do
{:qex, "~> 0.3"},
{:telemetry, "~> 1.0"},
{:bunch, "~> 1.5"},
{:ratio, "~> 2.0"},
{:ratio, "~> 3.0"},
# Development
{:ex_doc, "~> 0.28", only: :dev, runtime: false},
{:makeup_diff, "~> 0.1", only: :dev, runtime: false},
Expand Down
4 changes: 2 additions & 2 deletions mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"dialyxir": {:hex, :dialyxir, "1.2.0", "58344b3e87c2e7095304c81a9ae65cb68b613e28340690dfe1a5597fd08dec37", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "61072136427a851674cab81762be4dbeae7679f85b1272b6d25c3a839aff8463"},
"earmark_parser": {:hex, :earmark_parser, "1.4.29", "149d50dcb3a93d9f3d6f3ecf18c918fb5a2d3c001b5d3305c926cddfbd33355b", [:mix], [], "hexpm", "4902af1b3eb139016aed210888748db8070b8125c2342ce3dcae4f38dcc63503"},
"erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
"ex_doc": {:hex, :ex_doc, "0.29.0", "4a1cb903ce746aceef9c1f9ae8a6c12b742a5461e6959b9d3b24d813ffbea146", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "f096adb8bbca677d35d278223361c7792d496b3fc0d0224c9d4bc2f651af5db1"},
"ex_doc": {:hex, :ex_doc, "0.29.1", "b1c652fa5f92ee9cf15c75271168027f92039b3877094290a75abcaac82a9f77", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "b7745fa6374a36daf484e2a2012274950e084815b936b1319aeebcf7809574f6"},
"excoveralls": {:hex, :excoveralls, "0.15.0", "ac941bf85f9f201a9626cc42b2232b251ad8738da993cf406a4290cacf562ea4", [:mix], [{:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "9631912006b27eca30a2f3c93562bc7ae15980afb014ceb8147dc5cdd8f376f1"},
"file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"},
"hackney": {:hex, :hackney, "1.18.1", "f48bf88f521f2a229fc7bae88cf4f85adc9cd9bcf23b5dc8eb6a1788c662c4f6", [:rebar3], [{:certifi, "~>2.9.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~>6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~>1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~>1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "a4ecdaff44297e9b5894ae499e9a070ea1888c84afdd1fd9b7b2bc384950128e"},
Expand All @@ -25,7 +25,7 @@
"numbers": {:hex, :numbers, "5.2.4", "f123d5bb7f6acc366f8f445e10a32bd403c8469bdbce8ce049e1f0972b607080", [:mix], [{:coerce, "~> 1.0", [hex: :coerce, repo: "hexpm", optional: false]}, {:decimal, "~> 1.9 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "eeccf5c61d5f4922198395bf87a465b6f980b8b862dd22d28198c5e6fab38582"},
"parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
"qex": {:hex, :qex, "0.5.1", "0d82c0f008551d24fffb99d97f8299afcb8ea9cf99582b770bd004ed5af63fd6", [:mix], [], "hexpm", "935a39fdaf2445834b95951456559e9dc2063d0a055742c558a99987b38d6bab"},
"ratio": {:hex, :ratio, "2.4.2", "c8518f3536d49b1b00d88dd20d49f8b11abb7819638093314a6348139f14f9f9", [:mix], [{:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}, {:numbers, "~> 5.2.0", [hex: :numbers, repo: "hexpm", optional: false]}], "hexpm", "441ef6f73172a3503de65ccf1769030997b0d533b1039422f1e5e0e0b4cbf89e"},
"ratio": {:hex, :ratio, "3.0.2", "60a5976872a4dc3d873ecc57eed1738589e99d1094834b9c935b118231297cfb", [:mix], [{:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}, {:numbers, "~> 5.2.0", [hex: :numbers, repo: "hexpm", optional: false]}], "hexpm", "3a13ed5a30ad0bfd7e4a86bf86d93d2b5a06f5904417d38d3f3ea6406cdfc7bb"},
"ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
"telemetry": {:hex, :telemetry, "1.1.0", "a589817034a27eab11144ad24d5c0f9fab1f58173274b1e9bae7074af9cbee51", [:rebar3], [], "hexpm", "b727b2a1f75614774cff2d7565b64d0dfa5bd52ba517f16543e6fc7efcc0df48"},
"unicode_util_compat": {:hex, :unicode_util_compat, "0.7.0", "bc84380c9ab48177092f43ac89e4dfa2c6d62b40b8bd132b1059ecc7232f9a78", [:rebar3], [], "hexpm", "25eee6d67df61960cf6a794239566599b09e17e668d3700247bc498638152521"},
Expand Down
8 changes: 4 additions & 4 deletions test/membrane/clock_test.exs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
defmodule Membrane.ClockTest do
use ExUnit.Case
use Numbers, overload_operators: true

@module Membrane.Clock

Expand All @@ -16,16 +17,15 @@ defmodule Membrane.ClockTest do
refute_receive {:membrane_clock_ratio, ^clock, _ratio}
send(clock, {:membrane_clock_update, 2})
send(clock, time: 13)
assert_receive {:membrane_clock_ratio, ^clock, 2}
two = Ratio.new(2, 1)
assert_receive {:membrane_clock_ratio, ^clock, ^two}
send(clock, {:membrane_clock_update, random_time()})
send(clock, time: 33)
ratio = Ratio.new(20 + 2, 33 - 3)
assert_receive {:membrane_clock_ratio, ^clock, ^ratio}
end

test "should handle different ratio formats" do
use Ratio

{:ok, clock} =
@module.start_link(time_provider: fn -> receive do: (time: t -> ms_to_ns(t)) end)

Expand All @@ -38,7 +38,7 @@ defmodule Membrane.ClockTest do
send(clock, {:membrane_clock_update, 5})
send(clock, time: 20)
@module.subscribe(clock)
ratio = (5 + Ratio.new(1, 3) * 2) / (20 - 5)
ratio = Ratio.div(5 + Ratio.new(1, 3) * 2, Ratio.new(20 - 5))
assert_receive {:membrane_clock_ratio, ^clock, ^ratio}
end

Expand Down