From 585fddceb835c482c4cba50d94d0d99a8186f32d Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" <feliks.pobiedzinski@swmansion.com> Date: Thu, 8 Dec 2022 15:48:20 +0100 Subject: [PATCH 1/4] Remove or rename Membrane.Time.round_to_* functions --- CHANGELOG.md | 4 ++++ lib/membrane/core/timer.ex | 2 +- lib/membrane/sync.ex | 2 +- lib/membrane/time.ex | 33 ++++++++++++++------------------- test/membrane/time_test.exs | 12 ++++++------ 5 files changed, 26 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 520548877..0e9449dbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 1.0.0 + * Replace `Membrane.Time.round_to_<unit_name>/1` with `Membrane.Time.as_<unit_name>/2` with second argument equal `:round`. + * Rename `Membrane.Time.round_to_timebase/2` to `Membrane.Time.divide_by_timebase/2`. + ## 0.11.0 * Separate element_name and pad arguments in handle_element_{start, end}_of_stream signature [#219](https://github.com/membraneframework/membrane_core/issues/219) * Refine communication between parent and its children [#270](https://github.com/membraneframework/membrane_core/issues/270) diff --git a/lib/membrane/core/timer.ex b/lib/membrane/core/timer.ex index 226c6e226..1da56708b 100644 --- a/lib/membrane/core/timer.ex +++ b/lib/membrane/core/timer.ex @@ -62,7 +62,7 @@ defmodule Membrane.Core.Timer do beam_next_tick_time = Ratio.add(Ratio.new(init_time), Ratio.div(next_tick_time, ratio)) |> Ratio.floor() - |> Time.round_to_milliseconds() + |> Time.as_milliseconds(:round) timer_ref = Process.send_after(self(), Message.new(:timer_tick, id), beam_next_tick_time, abs: true) diff --git a/lib/membrane/sync.ex b/lib/membrane/sync.ex index 9bd4955d6..be65a5c4f 100644 --- a/lib/membrane/sync.ex +++ b/lib/membrane/sync.ex @@ -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.round_to_milliseconds() + time = (max_latency - latency) |> Time.as_milliseconds(:round) Process.send_after(self(), {:reply, reply_to}, time) end) end diff --git a/lib/membrane/time.ex b/lib/membrane/time.ex index ec887e8f3..795464233 100644 --- a/lib/membrane/time.ex +++ b/lib/membrane/time.ex @@ -229,16 +229,16 @@ defmodule Membrane.Time do end @doc """ - Returns timestamp in timebase units. Rounded to the nearest integer. + Divides timestamp by timebase. Rounded to the nearest integer. ## Examples: iex> timestamp = 10 |> Membrane.Time.seconds() iex> timebase = Ratio.new(Membrane.Time.second(), 30) - iex> Membrane.Time.round_to_timebase(timestamp, timebase) + iex> Membrane.Time.divide_by_timebase(timestamp, timebase) 300 """ - @spec round_to_timebase(number | Ratio.t(), number | Ratio.t()) :: integer - def round_to_timebase(timestamp, timebase) do + @spec divide_by_timebase(number | Ratio.t(), number | Ratio.t()) :: integer + def divide_by_timebase(timestamp, timebase) do Ratio.new(timestamp, timebase) |> round_rational() end @@ -267,26 +267,21 @@ defmodule Membrane.Time do |> round_rational() end - round_to_fun_name = :"round_to_#{unit.plural}" - - @doc """ - Returns time in #{unit.plural}. Rounded to the nearest integer. - """ - @spec unquote(round_to_fun_name)(t) :: integer - # credo:disable-for-next-line Credo.Check.Readability.Specs - 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}" @doc """ - Returns time in #{unit.plural}, represented as a rational number. + Returns time in #{unit.plural}. If second argument is not provided + or is equal to `:exact`, result is represented as a rational number. + If second argument is equal to `:round`, result is rounded to the + nearest integer. """ - @spec unquote(as_fun_name)(t) :: integer | Ratio.t() + @spec unquote(as_fun_name)(t, :round | :exact) :: integer | Ratio.t() # credo:disable-for-next-line Credo.Check.Readability.Specs - def unquote(as_fun_name)(time) when is_time(time) do - Ratio.new(time, unquote(unit.duration)) + def unquote(as_fun_name)(time, mode \\ :exact) when is_time(time) do + case mode do + :exact -> Ratio.new(time, unquote(unit.duration)) + :round -> Ratio.new(time, unquote(unit.duration)) |> round_rational() + end end end) diff --git a/test/membrane/time_test.exs b/test/membrane/time_test.exs index f25dd898f..9d6b7c14a 100644 --- a/test/membrane/time_test.exs +++ b/test/membrane/time_test.exs @@ -66,11 +66,11 @@ defmodule Membrane.TimeTest do end test "Time.to_timebase/2 works properly" do - assert @module.round_to_timebase(4, 2) == 2 - assert @module.round_to_timebase(3, Ratio.new(3, 2)) == 2 - assert @module.round_to_timebase(Ratio.new(15, 2), 2) == 4 - assert @module.round_to_timebase(Ratio.new(15, 2), Ratio.new(3, 2)) == 5 - assert @module.round_to_timebase(4, 10) == 0 - assert @module.round_to_timebase(4, 7) == 1 + assert @module.divide_by_timebase(4, 2) == 2 + assert @module.divide_by_timebase(3, Ratio.new(3, 2)) == 2 + assert @module.divide_by_timebase(Ratio.new(15, 2), 2) == 4 + assert @module.divide_by_timebase(Ratio.new(15, 2), Ratio.new(3, 2)) == 5 + assert @module.divide_by_timebase(4, 10) == 0 + assert @module.divide_by_timebase(4, 7) == 1 end end From a2658fc2f0fd55734731780115e94e27a94f633c Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" <feliks.pobiedzinski@swmansion.com> Date: Thu, 8 Dec 2022 15:52:57 +0100 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e9449dbd..ae3a6c7c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,7 @@ # Changelog ## 1.0.0 - * Replace `Membrane.Time.round_to_<unit_name>/1` with `Membrane.Time.as_<unit_name>/2` with second argument equal `:round`. - * Rename `Membrane.Time.round_to_timebase/2` to `Membrane.Time.divide_by_timebase/2`. + * Replace `Membrane.Time.round_to_<unit_name>/1` with `Membrane.Time.as_<unit_name>/2` with second argument equal `:round`. Rename `Membrane.Time.round_to_timebase/2` to `Membrane.Time.divide_by_timebase/2` [#494](https://github.com/membraneframework/membrane_core/pull/494) ## 0.11.0 * Separate element_name and pad arguments in handle_element_{start, end}_of_stream signature [#219](https://github.com/membraneframework/membrane_core/issues/219) From bd21025849c6a6f5e716cf212828862844b2f2ab Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" <feliks.pobiedzinski@swmansion.com> Date: Wed, 14 Dec 2022 10:12:35 +0100 Subject: [PATCH 3/4] Fix typos and warnings from docs --- CHANGELOG.md | 2 +- lib/membrane/time.ex | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae3a6c7c3..72e440b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog ## 1.0.0 - * Replace `Membrane.Time.round_to_<unit_name>/1` with `Membrane.Time.as_<unit_name>/2` with second argument equal `:round`. Rename `Membrane.Time.round_to_timebase/2` to `Membrane.Time.divide_by_timebase/2` [#494](https://github.com/membraneframework/membrane_core/pull/494) + * Replace `Membrane.Time.round_to_<unit_name>` with `Membrane.Time.as_<unit_name>/2` with second argument equal `:round`. Rename `Membrane.Time.round_to_timebase` to `Membrane.Time.divide_by_timebase/2` [#494](https://github.com/membraneframework/membrane_core/pull/494) ## 0.11.0 * Separate element_name and pad arguments in handle_element_{start, end}_of_stream signature [#219](https://github.com/membraneframework/membrane_core/issues/219) diff --git a/lib/membrane/time.ex b/lib/membrane/time.ex index 795464233..4b2d76676 100644 --- a/lib/membrane/time.ex +++ b/lib/membrane/time.ex @@ -229,7 +229,7 @@ defmodule Membrane.Time do end @doc """ - Divides timestamp by timebase. Rounded to the nearest integer. + Divides timestamp by a timebase. The result is rounded to the nearest integer. ## Examples: iex> timestamp = 10 |> Membrane.Time.seconds() @@ -270,10 +270,10 @@ defmodule Membrane.Time do as_fun_name = :"as_#{unit.plural}" @doc """ - Returns time in #{unit.plural}. If second argument is not provided - or is equal to `:exact`, result is represented as a rational number. - If second argument is equal to `:round`, result is rounded to the - nearest integer. + Returns time in #{unit.plural}. The default value of the second argument + is `:exact`. If the second argument is equal to `:exact`, the result is + represented as a rational number. Otherwise, if the second argument is + equal to `:round`, the result is rounded to the nearest integer. """ @spec unquote(as_fun_name)(t, :round | :exact) :: integer | Ratio.t() # credo:disable-for-next-line Credo.Check.Readability.Specs From aa2a99e7ae68155380720cef7eaabefb4f05d1d6 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" <feliks.pobiedzinski@swmansion.com> Date: Thu, 15 Dec 2022 11:26:05 +0100 Subject: [PATCH 4/4] Update docs due to suggestions from PR --- CHANGELOG.md | 2 +- lib/membrane/time.ex | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e4d1b622..7e1dddaa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,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. Add `Membrane.Time.round_to_timebase/2` function. + * 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. Add `Membrane.Time.round_to_timebase` function. * New `spec` action syntax - the structure of pipeline is now defined with the use of `Membrane.ChildrenSpec` * Rename `:caps` to `:stream_format`. * Use Elixir patterns as `:accepted_format` in pad definition. diff --git a/lib/membrane/time.ex b/lib/membrane/time.ex index 4b2d76676..d8d27a6b0 100644 --- a/lib/membrane/time.ex +++ b/lib/membrane/time.ex @@ -270,12 +270,13 @@ defmodule Membrane.Time do as_fun_name = :"as_#{unit.plural}" @doc """ - Returns time in #{unit.plural}. The default value of the second argument - is `:exact`. If the second argument is equal to `:exact`, the result is - represented as a rational number. Otherwise, if the second argument is - equal to `:round`, the result is rounded to the nearest integer. + Returns time in #{unit.plural}. + + If the `mode` argument is set to `:exact` (default), the result is + represented as a rational number. Otherwise, if the `mode` is + `:round`, the result is rounded to the nearest integer. """ - @spec unquote(as_fun_name)(t, :round | :exact) :: integer | Ratio.t() + @spec unquote(as_fun_name)(t, mode :: :round | :exact) :: integer | Ratio.t() # credo:disable-for-next-line Credo.Check.Readability.Specs def unquote(as_fun_name)(time, mode \\ :exact) when is_time(time) do case mode do