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

Fix spatial layer switch with unordered packets #823

Merged
merged 7 commits into from
Jun 3, 2022

Conversation

jcague
Copy link
Contributor

@jcague jcague commented May 17, 2022

I've seen sporadic corrupted frames when debugging VP8 Simulcast locally. I recorded a couple of cases here:
https://user-images.githubusercontent.com/557698/168805998-dada1fc0-a47b-4383-b46d-d0a0052fc342.mov
and here
https://user-images.githubusercontent.com/557698/168805983-95f2942c-fd92-4a6c-b89b-d2315ff0f41d.mov

In these cases I saw the same pattern:

We forward frames belonging to layer 0 following their sequence number, but there is a sequence number gap in the target spatial layer (due to packet losses or reorder) just before the keyframe that acts as the sync packet. In that case, if we receive the packets that were previously lost we will forward them to the consumer, and the browser will render a broken frame.

The impact is quite limited as it usually ends up showing just a corrupted frame.

This is what happens to the traffic:

<-- Current Spatial Layer 0, Target Spatial Layer 0
| SN 0   | SL 0 | M |    | --> forwarded as SN 0
| SN 100 | SL 1 | M |    | --> dropped (different SL)
| SN 1   | SL 0 | M |    | --> forwarded as SN 1
| SN 101 | SL 1 | M |    | --> lost
<-- Current Spatial Layer 0, Target Spatial Layer 1
| SN 2   | SL 0 |   | KF | --> forwarded as SN 2
| SN 102 | SL 1 |   | KF | --> forwarded as SN 4 (previous packet did not include marker)
| SN 101 | SL 1 |   |    | --> forwarded as SN 3 and this is usually decoded as it meets all requirements (SN, PID, etc.)

SN = Sequence Number, SL = Spatial Layer, KF = keyframe, and M = marker bit set

As a result, the browser will decode:

Frame from packet SN 0, SL0
Frame from packet SN 1, SL0
Frame from packet SN 2, SL0, start of a keyframe
       and packet SN 3, SL1, that was retransmitted and belonged to a different layer, possibly causing a corrupted frame
Frame from packet SN 4, SL1, keyframe

Here I propose a solution to avoid sending packets with sequence numbers lower than the first keyframe which was the sync packet.

@ibc
Copy link
Member

ibc commented Jun 2, 2022

@jmillan any other concern here? Ready to merge?

@ibc ibc requested a review from jmillan June 2, 2022 13:15
@jmillan
Copy link
Member

jmillan commented Jun 2, 2022

I'll check it tomorrow.

Copy link
Member

@jmillan jmillan left a comment

Choose a reason for hiding this comment

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

👍

@ibc ibc merged commit 99f9cbf into versatica:v3 Jun 3, 2022
@ibc
Copy link
Member

ibc commented Jun 3, 2022

Thanks. Released in mediasoup 3.9.16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants