Rtcp sender packet event #393
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #393 +/- ##
==========================================
+ Coverage 54.95% 55.22% +0.27%
==========================================
Files 75 75
Lines 3259 3299 +40
==========================================
+ Hits 1791 1822 +31
- Misses 1468 1477 +9
Continue to review full report in Codecov by Sentry.
|
Karolk99
force-pushed
the
rtcp-sender-packet-event
branch
from
ae8cf1f
to
5c3eafe
Compare
recording/lib/reporter.ex
Outdated
|
|
||
| defp add_wallclock_start_time(track, rtcp) do | ||
| delta_t_ns = | ||
| (rtcp.sender_info.rtp_timestamp - track.start_timestamp) / track.clock_rate * 10 ** 9 |
There was a problem hiding this comment.
Could you move this number to constant?
recording/lib/reporter.ex
Outdated
| defp recalculate_offsets(tracks) do | ||
| {tracks, _acc} = | ||
| tracks | ||
| |> Enum.sort_by(fn {_filename, track} -> track.offset end) | ||
| |> Enum.map_reduce(nil, fn {filename, track}, acc -> | ||
| cond do | ||
| not Map.has_key?(track, :start_timestamp_wallclock) -> | ||
| {{filename, track}, acc} | ||
|
|
||
| is_nil(acc) -> | ||
| {{filename, track}, track} | ||
|
|
||
| true -> | ||
| offset = acc.offset + track.start_timestamp_wallclock - acc.start_timestamp_wallclock | ||
| {{filename, %{track | offset: offset}}, acc} | ||
| end | ||
| end) | ||
|
|
||
| tracks | ||
| end |
There was a problem hiding this comment.
Some comments are needed about how this synchronization works and why we do it that way.
Karolk99
force-pushed
the
rtcp-sender-packet-event
branch
from
967ff50
to
d89dd03
Compare
There was a problem hiding this comment.
I think we could add another test where only two tracks have RTCP reports but they are not in order e.g: second and fourth will have RTCP reports.
|
|
||
| events = | ||
| for _idx <- 0..events do | ||
| {:event, {:output, %RTCPEvent{rtcp: rtcp}}} |
There was a problem hiding this comment.
This way, we don't know if the last RTCPEvent is saved or the last one, as we generate five of the same events.
| @@ -166,4 +171,150 @@ defmodule Membrane.RTC.Engine.Endpoint.Recording.ReporterTest do | |||
| } | |||
| } = Reporter.get_report(reporter) | |||
| end | |||
|
|
|||
| describe "RTCP synchronization" do | |||
There was a problem hiding this comment.
I would add some comments about how this test should work for future contributors, as it is not obvious how each of these tests works.
No description provided.