For a participant migrating out, the track could be resumed on a
different node, but ending on the migrating out node. So, `flush` should
be used to indicate if track is going to be resumed.
Removing some logs which have not been useful in terms of insights other
than saying that there are a bunch of packets missing. Going to start
looking at gaps in terms of time if the inter-packet gap is too high.
Also, using logging these events as first 20 and then every 200.
A bunch of edges to note here
RED packet does not have sequence number for redundant blocks. It only
has timestamp offset compared to the primary payload. The receivers are
supposed to use just timestamp to sequence the payload and decode.
But, when converting from RED -> Opus, the packets extracted from RED
packet should be assigned a sequence number before they can be
forwarded. The simple rule is, if packet N contains X redundant
payloads, they are assigned sequence number of N - X to N - 1.
However there are cases like the following sequence (with 1 packet
redundancy)
- Seq num 10, timestamp 2000, forwarded
- Seq num 11 is lost
- Seq num 12 has a redundant payload. Seq num 12 has timestamp of 4000.
Ideally would expect the redundant payload to have a timestamp offset
of 1000, so the redundant payload can be mapped to sequence number 11
and timestamp 3000 (4000 - 1000). But, in the problematic case, it has
an offset of 3000 resulting in sequence number 11 and timestamp of
1000 causing an inversion with packet at sequence number 10.
Unclear if this a publisher issue, i. e. packing RED wrong or if this is
some expected behaviour with DTX. i. e. the DTX packets are not included
in redundant payload. For example, the sequence
- Seq num 10 -> DTX
- Seq num 11 -> DTX -> lost
- Seq num 12 -> Regular packet and include sequence num 9 as that is the
last regular packet.
Anyhow, detect this condition and drop the time inverted packet.
Note however this handles only inversion against the highest sent packet
sequence number and timestamp. So, some old packet inverted with some
other old packet getting forwarded will get through. That has been the
case always though and detecting that would be expensive and
complicated.
At least for egress, will also look at adding a check for inversion so
that it can catch it before sending it down the gstreamer pipeline. As
the egress uses a jitter buffer with ordered sequence number emits, it
will be simpler to detect timestamp going back when sequence number is
moving forward (of course the mute/dtx challenege is there).
* Log time inversion between incoming packets
Log of timestamp inversion within a red packet did not show anything.
Log across packets. Not dropping till there is more evidence of the
cause.
* save
* comment
* Guard against timestamp inversion in RED -> Opus conversion.
Seeing timestamp inversion (sequence number is +1, but timestamp is
-960, i.e. 20ms) in the RED -> Opus conversion path. Not able to spot
any bugs in code. So, logging details upon detection and also dropping
the packet. If not dropped, downstream components like Egress treat it
as a big timestamp jump (because sequence number is moving forward) and
try to adjust pts which ends up causing drops.
* do not log time reversal at the start
* typo
* Add debug for receiver restart.
Have a suspicion that something is deadlocking between restart receiver
and buffer bind during replay. Adding debug to get a better picture of
state of receiver restart.
* consistent logging
* Optimise some bits in rtpstats_receiver
RTPStatsReceiver.Update is one of the high CPU bits. Taking some
suggestions from Cursor. Makes the `Update` function verbose though :-(
* zap.Inline logging fields
* rename
* Set up audio config in audio level module when config is updated.
It is possible to get audio config after bind (bind is where the audio
level module is created) for remote tracks. So, split out setting audio
level config in audio level module and invoke it when config is updated.
* coderabbit review
* prevent divide-by-0
* not active before config
* Do not increase max expected layer on track info update.
When max expected layer increases, the corresponding trackers are reset
so that first packets from those layers can trigger a layer detected
change enabling quick detection of layer start.
A track info update changing max to what is in track info could set the
max expected to be higher without resetting the tracker. And that would
cause dynacast induced max layer change to miss tracker reset too.
Sequence
- dynacast sets max expected to 0
- track info update sets it to 2
- dynacast sets it to 1 --> this should have reset tracker on layer 1,
but because it is less than current max (2), it is skipped.
* thank you CodeRabbit
* force update on start
* Reducing some info level logs.
Also, relaxing the check for runaway RTCP receiver report to allow for
rollover to catch up if it is not too far away.
* set logger
With audio simulcast codecs, it is possible that the clock rate of the
primary codec is different from the secondary codec. If a subscriber
binds to the secondary codec, the clock rate should be set correctly. Do
it at bind time.
* Do not restart receiver on codec change mid-session.
This is not supported and was an erroneous change during the
receiver_base/buffer_base + RTP stream restart consolidation. Also make
the codec munger creation more resilient.
* fix test
In some paths, it is better to preserve pre-recorded time. So, make the
base implementations preserve the RTCP Sender Report receive time.
Also, add a method to enable forwarding packet arrival time. Could be
used across relay.
* Rework receiver restart.
- Protect against concurrent restarts
- Clean up and consolidate code around restarts
- Use `RestartStream` of buffer rather than creating new buffers.
* fix test
* Minor refactor in buffer base and audio level
- Make a function for `restartStream`. Will be useful
when external signal needs to restart a stream. Also restart all the
bits (audio level, dd parser and frame rate calculator)
- make an audio level mode with RTP timestamp so that some state can be
moved out of buffer base
* clean up
* log restart
