Commit Graph
362 Commits
Author SHA1 Message Date
Raja Subramanian 99601e6d41 Handle the case of no packets in down stream tracks better. (#1500) 2023-03-07 14:32:43 +05:30
Raja Subramanian d2e7818eca Do not enable bitrate based scoring for screen share. (#1497) 2023-03-07 09:59:10 +05:30
Raja Subramanian 04269c100c Connection quality misc changes (#1496)
* Connectino quality misc changes

1. Call scorer.Update() with nil stat when no data available so that
   scorer can synthesise window with proper window time.
2. Substract out loss in interval to account for packets not sent at
   all.
3. Fix `packetsNotFound` variable in `getIntervalStats`. I remember this
   working at some point. Not sure if I fat fingered in another PR and
   deleted the increment line.
4. Logging a bit more when no packets expected. Those can get noisy
   especially when track is muted. But, seeing some unexplained
   instances of no packets leading to quality drop. So, temporary logging
   to get a bit more information.

* correct spelling

* Limit packet score minimum to 0.0
2023-03-07 09:08:19 +05:30
Raja Subramanian 15eae2119c prevent data race (#1492) 2023-03-05 17:32:56 +05:30
Raja Subramanian ea1a467191 Bitrate based quality tracking for DownTrack (#1491)
* Make connection quality not too optimistic.

With score normalization, the quality indicator showed good
under conditions which should have normally showed some badness.

So, a few things in this PR
- Do not normalize scores
- Pick the weakest link as the representative score (moving away from
  averaging)
- For down track direction, when reporting delta stats, take the number
  of packets sent actually. If there are holes in the feed (upstream
  packet loss), down tracks should not be penalised for that loss.

State of things in connection quality feature
- Audio uses rtcscore-go (with a change to accommodate RED codec). This
  follows the E-model.
- Camera uses rtcscore-go. No change here. NOTE: THe rtscore here is
  purely based on bits per pixel per frame (bpf). This has the following
  existing issues (no change, these were already there)
  o Does not take packet loss, jitter, rtt into account
  o Expected frame rate is not available. So, measured frame rate is
    used as expected frame rate also. If expected frame rate were available,
    the score could be reduced for lower frame rates.
- Screen share tracks: No change. This uses the very old simple loss
  based thresholding for scoring. As the bit rate varies a lot based on
  content and rtcscore video algorithm used for camera relies on
  bits per pixel per frame, this could produce a very low value
  (large width/height encoded in a small number of bits because of static content)
  and hence a low score. So, the old loss based thresholding is used.

* clean up

* update rtcscore pointer

* fix tests

* log lines reformat

* WIP commit

* WIP commit

* update mute of receiver

* WIP commit

* WIP commit

* start adding tests

* take min score if quality matches

* start adding bytes based scoring

* clean up

* more clean up

* Use Fuse

* log quality drop

* Periodically report bitrate to down track.

For connection quality based on bitrate for down tracks,
the measured rate should be used. That is to ensure that
down track quality measurement does not get affected by
publisher side changes negatively (or positively).
Report the optimal bit rate to connection quality scorer
every second so that scorer has a continuously updating
picture of the stream and can compare the actual bit rate
against expected optimal bitrate more reliably.

Doing it at time like allocation, the bitrate may not be
accurate (or may not even be available). So, a periodic update
is necessary.

* add transition at allocation times

* clean up debug log

* - Use number of windows for wait to make things simpler
- track no layer expected case
- always update transition
- always call updateScore
2023-03-05 14:10:19 +05:30
Raja Subramanian 9e327b1f3c Connection quality (#1490)
* Make connection quality not too optimistic.

With score normalization, the quality indicator showed good
under conditions which should have normally showed some badness.

So, a few things in this PR
- Do not normalize scores
- Pick the weakest link as the representative score (moving away from
  averaging)
- For down track direction, when reporting delta stats, take the number
  of packets sent actually. If there are holes in the feed (upstream
  packet loss), down tracks should not be penalised for that loss.

State of things in connection quality feature
- Audio uses rtcscore-go (with a change to accommodate RED codec). This
  follows the E-model.
- Camera uses rtcscore-go. No change here. NOTE: THe rtscore here is
  purely based on bits per pixel per frame (bpf). This has the following
  existing issues (no change, these were already there)
  o Does not take packet loss, jitter, rtt into account
  o Expected frame rate is not available. So, measured frame rate is
    used as expected frame rate also. If expected frame rate were available,
    the score could be reduced for lower frame rates.
- Screen share tracks: No change. This uses the very old simple loss
  based thresholding for scoring. As the bit rate varies a lot based on
  content and rtcscore video algorithm used for camera relies on
  bits per pixel per frame, this could produce a very low value
  (large width/height encoded in a small number of bits because of static content)
  and hence a low score. So, the old loss based thresholding is used.

* clean up

* update rtcscore pointer

* fix tests

* log lines reformat

* WIP commit

* WIP commit

* update mute of receiver

* WIP commit

* WIP commit

* start adding tests

* take min score if quality matches

* start adding bytes based scoring

* clean up

* more clean up

* Use Fuse

* log quality drop

* clean up debug log

* - Use number of windows for wait to make things simpler
- track no layer expected case
- always update transition
- always call updateScore
2023-03-05 12:55:04 +05:30
Raja Subramanian e48c818532 Resync on pub muted for audio to avoid jump in sequence numbers on (#1487)
unmute.
2023-03-03 12:18:25 +05:30
Raja Subramanian a35eecd03d Fix a case of changing video quality not succeeding. (#1483)
In the following order, got the wrong layer
- Max layer is 0, max published is 0, request layer is 0
- Current locks to 0.
- Max changes to 1. Nothing changes as 1 is not published yet.
- Max published changes to 1.
- As curernt layer is valid, available and locked to request layer, it
  was kept. But, it should have checked if the request layer changed
  and updated accordingly.
2023-03-01 11:12:54 +05:30
Raja Subramanian ab098d951e Prevent PLI layer lock getting stuck. (#1481)
In the following scenario, PLI layer lock got stuck at the wrong layer
- target is at 2 to allow overshoot
- current gets to 1, but can't get higher because publisher is not
  publishing higher layer
- max layer changed to 0

Because of adjusting for overshoot only when current == target, it never
happened and layer lock PLI kept asking for layer 0. Although, key
frames were received, switch did not happen.

Always check for overshoot adjustment possibility against current layer.
2023-03-01 06:35:41 +05:30
Raja Subramanian e423873aa3 Do not include packet in RED if timestamp is too far back. (#1478)
* Do not include packet in RED if timestamp is too far back.

* add test case for large timestmap jump
2023-02-28 10:13:42 +05:30
Raja Subramanian 8e6bcdaffe Change lock scope of access to RTCP sender report data. (#1473)
* Change lock scope of access to RTCP sender report data.

Forwarder calls back to get time stamp offset.
Holding buffer lock is a much bigger scoped lock.

Reduce lock scope and cache latest sender report under its own lock.
And use that cache when calculating time stamp offset.

* move sr cache to stream tracker manager for re-use in relay

* cache before spread
2023-02-27 12:28:25 +05:30
Raja Subramanian 207c97c4b9 Max published layer callback in goroutine (#1472) 2023-02-27 09:20:27 +05:30
David Zhao 34fcf9e496 Additional case of subscribing to a closed track (#1465)
When the publisher stops publishing, the individual receivers would close
attached DownTracks first before notifying MediaTrackReceiver callbacks.

This means #1454 does not fix the issue entirely since there is still a window
when we can subscribe to a closing track.
2023-02-23 17:07:16 -08:00
Raja Subramanian 15232560bc Send stream start on initial start (#1456)
A couple of other bits
1. Use request layer for sending PLI on bind and connected.
2. When adjusting for overshoot, do not adjust target unless current is
   at max. If not, it could get stuck in a lower layer in the following
   scenario
      a. Overshoot to layer 2
      b. Max layer is 1, start sending PLI
      c. Get key frame for layer 0, adjust for overshoot as we have
         something at a layer lower than max.
      d. Adjust for overshoot.
      e. Setting target to max means that current and target are equal
         and no further adjustment happens.
2023-02-23 09:35:12 +05:30
cnderrauber 1b3d6fad54 update to utils parallel execute (#1450)
* log change

* remove identity field

* update to protocol parallel execute

* update protocol
2023-02-22 11:09:32 +08:00
Raja Subramanian 6f326be4f7 Do not overshoot when layer is locked. (#1449)
* Do not overshoot when layer is locked.

One more challenging case. When current layer is already locked,
should not set up for overshoot.

* set target to current
2023-02-20 16:41:58 +05:30
Raja Subramanian d1ce60192c Handle case of layer stop with out bitrate. (#1448)
Missed this in the last commit. Sorry.
The case of
- locking to a layer
- that layer stops
- re-allocation

This should trigger a key frame request to the max available layer.
So, have to set the request layer to max available.
2023-02-20 14:49:32 +05:30
Raja Subramanian c51d083016 Handle edge case of no published layer (#1446)
* WIP commit

* set max published layer

* split target and request layer
2023-02-20 14:16:46 +05:30
Raja Subramanian 6b55742564 Use available layers in optimal allocation. (#1445)
Addressing edge case where a layer stopped before bitrate could be
measured. Purely bit rate based change deduction missed this as
the before and after did not have bit rates.

Use available layers to look for changes, especially currently
forwarding layer going away.

Also, simplifying bits. Only in the optimal allocation path,
these things are required. When congested, bitrate is always needed.
So, for optimal path, just look at available layer changes and adjust.

Don't need to look for bitrate based layer changes. Clean up that code.
2023-02-19 11:41:40 +05:30
Raja Subramanian 0dcd4e4856 Ensure temporal is not at -1 for non-simulcast streams (#1441) 2023-02-18 09:52:53 +05:30
Raja Subramanian 67b259bda4 consistent logging (#1437) 2023-02-17 23:41:58 +05:30
Raja Subramanian 9bf80511a9 Check for valid layers and also log unexpected target (#1438) 2023-02-17 23:39:06 +05:30
Raja Subramanian 85a23bfffc Cleaning up availableLayers and exemptedLayers (#1407)
* WIP commit

* Send stream state paused only when it is paused due to bandwidth limitation.

When stream is resumed after a stream is paused, an active update is
sent. Note that this means if there are intervening events like
mute/unmute between pause and resume, resume will be sent.

* WIP commit

* fix compile

* WIP commit

* fixing tests

* clean up exempted layers

* clean up unused stuff

* correct comment

* Don't need ops queue as order is not important now

* static check

* kick off allocation when callbacks are set up, calling from receiver means callbacks may not be set up
2023-02-17 13:53:11 +05:30
cnderrauber 30834791fa Fix data stats bitrate calculate (#1432) 2023-02-17 12:33:28 +08:00
Raja Subramanian 610c37f51a Log switch point of feed SSRC for debugging. (#1423) 2023-02-15 12:34:40 +05:30
Raja Subramanian 3309c6fff0 Log feed SSRC (#1419) 2023-02-14 18:05:20 +05:30
Raja Subramanian 7857af4a66 Send unpublished callback unconditionally. (#1418)
Even if an add track has been queued and can be used immediately
when the previous incarnation unpublishes, send the unpublished
callback as the track was technically unpublished and republished.
The re-publish will pick the same track SID when the pending track
is queued as it will get the SID from an existing published track.
2023-02-14 13:17:54 +05:30
Trey Hakanson ce07914e44 Allow for strict ACKs to be disabled or subscriber peer connections (#1410) 2023-02-10 22:51:03 +05:30
Raja Subramanian 8a7478fd0e Send stream state paused only when it is paused due to bandwidth limitation. (#1391)
* WIP commit

* Send stream state paused only when it is paused due to bandwidth limitation.

When stream is resumed after a stream is paused, an active update is
sent. Note that this means if there are intervening events like
mute/unmute between pause and resume, resume will be sent.
2023-02-08 20:03:32 +05:30
Raja Subramanian 95bc5b9849 Trigger max layer switch when downgrading. (#1398)
When downgrading (could be due to overshoot or opportunistically
locking to a higher layer), need to check if the max layer notification
needs to be done so that dynacast has the right max layer for the
participant corresponding to this downtrack.
2023-02-08 18:54:54 +05:30
cnderrauber a04e7ce647 Fix panic by CreateSenderReport before bind completed (#1397) 2023-02-08 17:02:19 +08:00
Raja Subramanian a1338b149c Limit layer to num advertised. (#1395)
* Limit layer to num advertised.

Otherwise, this leads to invalid layer for non-simulcast tracks.

* need to subtrack 1

* fix test
2023-02-08 11:28:32 +05:30
cnderrauber 2393a559c4 ensuare onPacket is not nil in rtcpreader callback (#1390) 2023-02-07 15:49:32 +08:00
cnderrauber 524e8985c6 silent frame for muted audio downtrack (#1389)
send silent frame for subscribe to a muted audio downtrack (opus)
write blank frames for red audio track
2023-02-07 15:29:38 +08:00
cnderrauber 470a6fc9f1 Revert "Send silent frame for muted audio downtrack (#1385)" (#1387)
This reverts commit 0e699d6ed0.
2023-02-07 14:10:39 +08:00
cnderrauber 0e699d6ed0 Send silent frame for muted audio downtrack (#1385)
* Send silent frame for muted audio downtrack

* Don't drop audio packets on pub muted

* don't inject blank audio frames for muted track
2023-02-07 13:12:50 +08:00
Raja Subramanian 1c697a073c Re-initialise reference layer on a resume. (#1376)
* Re-initialise reference layer on a resume.

It is possible for a down track to resume from seeded state.
But, storing reference layer means it is storing something that
is dependent on up track. On a resume, if the up track does not
produce the reference layer, RTCP sender reports will not be sent.
So, remove that up track dependency and re-initialise the reference
layer on a resume.

This could affect A/V sync, but it should not be bad.
But, need to observe the effects.

* min difference of 1
2023-02-03 10:38:49 +05:30
David Zhao 15edd4d86e Fix potentially nil access in buffer (#1374) 2023-02-02 14:56:07 -08:00
Raja Subramanian ffadb94e3a Simplifying forwarding logic a bit (#1349)
* Notes on wht to do

- Should targetLayers be altered while doing opportunistic locking
- Should targetLayers be altered in any other path than stream allocator path?
- Lock to layer as long as it is <= opportunistic layer
- When not congested, opportunistic can be highest
- When congested, opportunistic could be nil or lowest if paused is not allowed
- When muting, can we hold on to current layers (or keep it as previous) and
  restore on unmute.
- Store current/target in forwarder state and restore on seeding
- Watch for looking for targetLayers, etc. when looking to insert padding
  packets. There may be an assumption about restarting on key frame and hence
  okay to insert padding when target layers are invalid. This may not be true
  any more when doing opportunistic forwarding.
- Can we distinguish between publisher mute or dynacast (i. e. publisher side
  stopping) vs subscriber mute and do something useful? Publisher side mute
  could mean continuity in sequence numbers on a restart (might be able to
  catch it with opportunistic forwarding). But, there is the challenge of
  unmute from publisher via signalling channel vs media. If media is arriving,
  should subscribers do opportunistic forwarding before publisher mute state
  update happens?
- Maybe introduce a mode where forwarding continues to a frame end (of course
  with a time limit just in case the end of frame packet is lost) and then
  insert silence/padding packets?
- Ensure that audio blank frame insertion does not suffer from frame boundary
  issues.

* pub/sub mute separate + more notes on things to check

* WIP commit, more notes

* WIP commit

* WIP commit

* WIP commit

* WIP commit

* WIP commit

* WIP commit

* clean up

* slightly better comments

* Do not stop on unmute

* do not inject blank frames when pub muted

* do not forward on audio publisher mute
2023-02-01 21:57:53 +05:30
Raja Subramanian 2671493870 Use purely RR based RTT. (#1351)
* Use purely RR based RTT.

With normalization of NTP time stamp to local time,
don't need to keep track of NTP time of publisher + local time of
when a report is sent. RTT calculations can happen with RR only.

Also, do not log errors when RTT cannot be calculated due to
no last SR. This can happen if the receiver sends an RR before
it receives an SR. As SFU is doing SRs once in 5 seconds, it is
possible some RRs happen before the first SR.

* use error type

* correct error name
2023-01-30 19:32:06 +05:30
cnderrauber 3a4a294a92 Enable upstream nack for opus only audio track (#1343) 2023-01-28 16:02:38 +08:00
Raja Subramanian c696626fe8 Use local time base for NTP in RTCP Sender Report for downtracks. (#1321)
* Use local time base for NTP in RTCP Sender Report for downtracks.

More details in comments in code.

* Remove debug

* RTCPSenderReportInfo -> RTCPSenderReportDataExt

* Get rid of sender report data pointer checks
2023-01-25 11:00:15 +05:30
Raja Subramanian 56803a6b29 No need for using arrival time diff. (#1318)
From logs, can see when reference layer is out of order, it is
out of order by a small amount most of the time (saw a maximum
of 5 ms in a sampling of logs). When using arrival time in those
case, some times the offset comes out to 10 ms or so. In most
of the cases, the time based diff is a lot higher (by several ms).
Just use the default of +1 diff on switch layer in case of
out-of-order time stamp. That will allow playback to move
forward and keep the timing close to actual frame time.
2023-01-23 10:17:37 +05:30
Raja Subramanian b7263b7625 Do not use local time stamp when sending RTCP Sender Report (#1315)
* some additional logging

* Do not use local time stamp when sending RTCP Sender Report

As local time does not take into account the transmission delay
of publisher side sender report, using local time to calculate
offset is not accurate.

Calculate NTP time stamp based on difference in RTP time.
Notes in code about some shortcomings of this, but should
get better RTT numbers. I think RTT numbers were bloated because of
using local time stamp.
2023-01-19 23:32:50 +05:30
Raja Subramanian 7aad888e4e Normalize NTP time base when calculating RTT (#1297)
* Normalize NTP time base when calculating RTT

* seed last SR
2023-01-11 12:25:14 +05:30
Raja Subramanian 69d879706d Use getBuffer consistently. (#1290)
* Use `getBuffer` consistently.

With SVC, it is possible that layer <-> buffer mapping is not 1:1.
So, use `getBuffer` method.

* fix recursive lock
2023-01-07 16:16:17 +05:30
Raja Subramanian 2b89c821ab An attempt to use publisher side RTCP sender report while forwarding (#1286)
* WIP commit

* comment

* clean up

* remove unused stuff

* cleaner comment

* remove unused stuff

* remove unused stuff

* more comments

* TrackSender method to handle RTCP sender report data

* fix test

* push rtcp sender report data to down tracks

* Need payload type for codec id mapping in relay protocol

* rename variable a  bit
2023-01-06 14:07:18 +05:30
cnderrauber fe4da84ee9 send padding packets of muted uptrack for go sdk (#1283) 2023-01-04 16:01:04 +08:00
Raja Subramanian cbc7d43ba4 Minor tweaks to frame based stream tracker. (#1281) 2023-01-03 18:16:55 +05:30
Raja Subramanian a87574da31 FPS based stream tracker tweaks (#1275)
* FPS based stream tracker tweaks

- Cleaning up code
- Two tweaks
  o A layer is declared active on receiving first packet (when starting fresh).
    But, if there are no frames after that (no packets after girst packet or
    there is only one frame), layer would not have been declared stopped as
    the previous version waited for second frame. Now, if there are no more
    frames in eval interval, declare the layer stopped.
  o When frame rate goes to 0, reset FPS calculator. Otherwise, layer starting
    after a long time will have frames spaced apart too far which would result
    in very low frame rate. Reset the calculator and let it pick up after the
    the layer restarts
- Also changing from lowest FPS -> estimated FPS and update up slowly and down fast.
  There are cases where frames are to far apart result in really low FPS. Seems to
  happen with NLC kind of cases where bandwidth is changed rapidly and the estimator
  on browser probably gets a bit confused and starts/stops layers a bit erratically.
  So, update estimate periodically to ensure eval interval is tracking current rate.

* fix factor

* spelling fix
2022-12-30 19:49:16 +05:30