Update Membrane.Time API #494
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
varsill
requested changes
Dec 12, 2022
lib/membrane/time.ex
Outdated
| @@ -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. | |||
There was a problem hiding this comment.
Suggested change
| Divides timestamp by timebase. Rounded to the nearest integer. | |
| Divides timestamp by a timebase. The result is rounded to the nearest integer. |
lib/membrane/time.ex
Outdated
Comment on lines
273
to
276
| 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. |
There was a problem hiding this comment.
I would avoid the statement if second argument is not provided and instead I would stick to saying that "the default value of the second argument is :exact"
mat-hek
reviewed
Dec 14, 2022
lib/membrane/time.ex
Outdated
Comment on lines
273
to
276
| 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. |
There was a problem hiding this comment.
Suggested change
| 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. |
lib/membrane/time.ex
Outdated
| """ | ||
| @spec unquote(as_fun_name)(t) :: integer | Ratio.t() | ||
| @spec unquote(as_fun_name)(t, :round | :exact) :: integer | Ratio.t() |
There was a problem hiding this comment.
Suggested change
| @spec unquote(as_fun_name)(t, :round | :exact) :: integer | Ratio.t() | |
| @spec unquote(as_fun_name)(t, mode :: :round | :exact) :: integer | Ratio.t() |
varsill
reviewed
Dec 15, 2022
| ## 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) | ||
| * 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. |
There was a problem hiding this comment.
Why are you modifying 0.11.0 changelog? The Membrane.Time.round_to_timebase/2 was in fact added then.
There was a problem hiding this comment.
I have only deleted /2 suffix. I have made this change to fix failing CI tests, eg:
mat-hek
approved these changes
Dec 15, 2022
varsill
approved these changes
Dec 20, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Jira ticket: https://membraneframework.atlassian.net/browse/MC-105