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

Add get_forwarded_tracks function #300

Merged
merged 8 commits into from
Aug 4, 2023

Conversation

Rados13
Copy link
Contributor

@Rados13 Rados13 commented Jul 21, 2023

No description provided.

@Rados13 Rados13 requested a review from sgfn July 21, 2023 09:25
@Rados13 Rados13 requested a review from mickel8 as a code owner July 21, 2023 09:25
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #300 (9bfd10d) into master (2d6d537) will increase coverage by 0.59%.
The diff coverage is 70.45%.

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
+ Coverage   51.63%   52.23%   +0.59%     
==========================================
  Files          36       39       +3     
  Lines        1925     1968      +43     
==========================================
+ Hits          994     1028      +34     
- Misses        931      940       +9     
Files Changed Coverage Δ
hls/test/support/file_source_endpoint.ex 82.75% <20.00%> (ø)
...ine/lib/membrane_rtc_engine/static_track_sender.ex 50.00% <50.00%> (ø)
engine/test/support/fake_source_endpoint.ex 85.71% <85.71%> (ø)
engine/test/support/fake_endpoint.ex 90.00% <90.00%> (ø)
engine/lib/membrane_rtc_engine/engine.ex 76.03% <100.00%> (+1.55%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d6d537...9bfd10d. Read the comment docs.

@Rados13 Rados13 requested a review from roznawsk July 24, 2023 07:54
@Rados13 Rados13 self-assigned this Jul 24, 2023
@doc """
Returns number of forwarded tracks in RTC Engine.
"""
@spec get_forwarded_tracks(rtc_engine :: pid()) :: integer()
Copy link
Member

Choose a reason for hiding this comment

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

[nitpick]

Suggested change
@spec get_forwarded_tracks(rtc_engine :: pid()) :: integer()
@spec get_forwarded_tracks_number(rtc_engine :: pid()) :: integer()

alternatively:
get_forwarded_track_count
get_num_forwarded_tracks

@Rados13 Rados13 force-pushed the feature/add-forwarded-tracks-metric branch from 21711f2 to 78000b5 Compare July 25, 2023 06:29
membrane_rtc_engine/lib/membrane_rtc_engine/engine.ex Outdated Show resolved Hide resolved
membrane_rtc_engine/mix.exs Outdated Show resolved Hide resolved
membrane_rtc_engine/test/support/file_endpoint.ex Outdated Show resolved Hide resolved
@Rados13 Rados13 requested a review from sgfn July 25, 2023 11:20
Copy link
Member

@sgfn sgfn left a comment

Choose a reason for hiding this comment

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

Nicely done! 🦭

Copy link
Contributor

@mickel8 mickel8 left a comment

Choose a reason for hiding this comment

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

Just one blocker, besides this LGTM!

membrane_rtc_engine/lib/membrane_rtc_engine/engine.ex Outdated Show resolved Hide resolved
membrane_rtc_engine/test/support/file_endpoint.ex Outdated Show resolved Hide resolved
membrane_rtc_engine/test/support/fake_endpoint.ex Outdated Show resolved Hide resolved
@Rados13 Rados13 force-pushed the feature/add-forwarded-tracks-metric branch from 45eeda3 to 9a070da Compare July 28, 2023 06:09
@Rados13 Rados13 requested a review from Karolk99 as a code owner July 28, 2023 06:09
@Rados13 Rados13 requested a review from mickel8 July 28, 2023 06:10
@mickel8 mickel8 removed their request for review July 28, 2023 10:47
@Rados13 Rados13 requested a review from mickel8 July 28, 2023 13:00
Copy link
Contributor

@mickel8 mickel8 left a comment

Choose a reason for hiding this comment

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

Could we wait until #306 is merged?

membrane_rtc_engine/test.logs Outdated Show resolved Hide resolved
membrane_rtc_engine_webrtc/mix.exs Outdated Show resolved Hide resolved
membrane_rtc_engine_hls/test/support/track_sender.ex Outdated Show resolved Hide resolved
@Rados13 Rados13 force-pushed the feature/add-forwarded-tracks-metric branch from 4cfb8b6 to dbc270f Compare August 3, 2023 08:25
@Rados13 Rados13 requested a review from mickel8 August 3, 2023 09:58
membrane_rtc_engine/test/support/fake_source_endpoint.ex Outdated Show resolved Hide resolved
membrane_rtc_engine/mix.exs Outdated Show resolved Hide resolved
@Rados13 Rados13 requested a review from mickel8 August 4, 2023 06:44
@Rados13 Rados13 force-pushed the feature/add-forwarded-tracks-metric branch from ed5ed6a to 631d905 Compare August 4, 2023 10:28
@Rados13 Rados13 merged commit 54dff5a into master Aug 4, 2023
33 checks passed
@Rados13 Rados13 deleted the feature/add-forwarded-tracks-metric branch August 4, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants