Skip to content
This repository was archived by the owner on Sep 19, 2024. It is now read-only.

Fix cropped share screen #256

Merged
merged 2 commits into from
Mar 22, 2023
Merged

Fix cropped share screen #256

merged 2 commits into from
Mar 22, 2023

Conversation

Karolk99
Copy link
Contributor

Copy link
Contributor

@daniel-jodlos daniel-jodlos left a comment

Choose a reason for hiding this comment

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

No idea what's going on here, just unlocking the PR. Please wait for a review from someone who at least has an idea of how to configure the compositor.

Comment on lines 14 to 15
@type track_no :: pos_integer()
@type padding :: pos_integer()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why pos_integer instead of non_neg_integer, don't you iterate tracks from 0?

@Karolk99 Karolk99 requested a review from Rados13 March 20, 2023 11:33
@Karolk99 Karolk99 force-pushed the ML-274-cropped-sharescreen branch from 792fc5e to 72bed29 Compare March 20, 2023 12:58
Comment on lines +506 to +509
# output_manifests =
# Enum.filter(output_files, fn file ->
# String.match?(file, ~r/\.m3u8$/) and file != @main_manifest
# end)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't use it shouldn't we remove it, or add some comments that this will be fixed in the future?

Copy link
Contributor

@Rados13 Rados13 left a comment

Choose a reason for hiding this comment

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

Also tests don't pass

@Rados13 Rados13 self-requested a review March 20, 2023 14:49
@Karolk99 Karolk99 force-pushed the ML-274-cropped-sharescreen branch 2 times, most recently from 462734b to eee53a2 Compare March 20, 2023 16:35
@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #256 (771b8a8) into master (fdd2f14) will decrease coverage by 0.05%.
The diff coverage is 28.57%.

@@            Coverage Diff             @@
##           master     #256      +/-   ##
==========================================
- Coverage   57.77%   57.73%   -0.05%     
==========================================
  Files          38       40       +2     
  Lines        1788     1791       +3     
==========================================
+ Hits         1033     1034       +1     
- Misses        755      757       +2     
Impacted Files Coverage Δ
...embrane_rtc_engine/endpoints/hls/desktop_layout.ex 0.00% <0.00%> (ø)
...membrane_rtc_engine/endpoints/hls/mobile_layout.ex 0.00% <0.00%> (ø)
...ne_rtc_engine/endpoints/hls/custom_layout_maker.ex 56.81% <33.33%> (ø)
test/support/test_layout_maker.ex 83.33% <100.00%> (ø)

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 fdd2f14...771b8a8. Read the comment docs.

@Karolk99 Karolk99 force-pushed the ML-274-cropped-sharescreen branch from eee53a2 to 771b8a8 Compare March 20, 2023 16:52
Comment on lines +268 to +269
assert_receive({:playlist_playable, :audio, ^output_dir}, 50_000)
assert_receive({:playlist_playable, :video, ^output_dir}, 50_000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 50 seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's how much time it takes when circle Ci servers are the slowest

@Karolk99 Karolk99 merged commit 5136fd8 into master Mar 22, 2023
@Karolk99 Karolk99 deleted the ML-274-cropped-sharescreen branch March 22, 2023 15:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants