AV1 issues investigation

[jmillan]

• Do not rewrite frame number in DD as the frame dependency template does reference the original ones.
• Include Active Decode Targets bitmask in each RTP packet as commented here .
• Include DD on RtpPacket->Dump()
• Implement logic to encode and decode payload (for retransmissions), adding the correct Active Decode Targets bitmask for each case.

Resources

https://aomediacodec.github.io/av1-rtp-spec

It's hateful when someone approves a experimental/testing PR. It's like "ok, good, approved, bye forever"

old review

[jmillan]

This PR is ready. Forwarding DD is now provides at least the same quality as not forwarding it.

[vpalmisano]

@jmillan After this change the piped AV1 consumers are still not working :(

[jmillan]

Not at all? I'll test it as soon as I can.

[vpalmisano]

Yep, no packets are delivered to the consumer on the piped router.
I'm trying to reproduce it with mediasoup-demo.

[ibc]

Just in case, are we negotiating DD extension in router.pipeToRouter()?

[vpalmisano]

It should, we use the sendrecv mode.

[vpalmisano]

Before the change made by @jmillan , I've proposed this one #1617 that works but probably it forwards everything from the producer to the receivers.

[ibc]

packet->Dump() in shows if DD header is present

[ibc]

And there is a C define in RtpPacket.cpp to make Dump() method print all the details of DD.

[jmillan]

Ok, here the summary of actions and rationale behind this PR.

Firstly AV1 capability was added here. It consisted of minimum DD parsing in order to get the spatial and temporal layers and frame number, basically. The RTP header extension was being explicitly left to recvonly, this is, mediasoup did not forward it.

This lead to the limitation of AV1 + DD not being able to be used in pipe transports.

In a second phase, as I was testing forwarding the DD, I realised of the problem with the freezes when forwarding the DD and that is why I created the corresponding mediasoup and chrome issues.

Upon investigating such issue, we starting rewriting the frameNumber of the DD to provide a continuous frameNumber to the receiver. Which I later realized it was wrong. We cannot alter the frameNumber because DD internally does contain a inter layer dependency relation based on the original frameNumber, so changing that would make all dependency structure incorrect. We also realized that webkit does not rewrite it.

To this point, frameNumber rewrite was removed, and following the AV1 payload spec we added to each packet the Active Decode Target Bitmask which basically indicates which layers are being sent, so the decoder knows at any time if a certain layer is no longer being sent.

Apart from writing such bitmask, we don't manipulate the packet, so if simply forwarding the packet works with pipe transports and doing that with the updated packets does not, then there must be obviously a problem there.

I would suggest we check that. This patch would remove the edition of the packet for AV1 + DD:

diff --git a/worker/src/RTC/Codecs/AV1.cpp b/worker/src/RTC/Codecs/AV1.cpp
index c3fbb7c10..da28d47a3 100644
--- a/worker/src/RTC/Codecs/AV1.cpp
+++ b/worker/src/RTC/Codecs/AV1.cpp
@@ -256,15 +256,6 @@ namespace RTC
                                context->SetCurrentTemporalLayer(tmpTemporalLayer);
                        }

-                       // Store the encoding data for retransmissions.
-                       // clang-format off
-                       this->payloadDescriptor->CreateEncoder({
-                         static_cast<uint32_t>(context->GetCurrentSpatialLayer()),
-                         static_cast<uint32_t>(context->GetCurrentTemporalLayer())
-                       });
-                       // clang-format on
-                       this->payloadDescriptor->Encode();
-
                        return true;
                }

diff --git a/worker/src/RTC/Producer.cpp b/worker/src/RTC/Producer.cpp
index 6dc6d6867..05e7a581e 100644
--- a/worker/src/RTC/Producer.cpp
+++ b/worker/src/RTC/Producer.cpp
@@ -1356,23 +1356,6 @@ namespace RTC
                                {
                                        std::memcpy(bufferPtr, extenValue, extenLen);

-                                       // Make place for the active decode target bitmask.
-                                       // clang-format off
-                                       if (
-                                         (packet->HasOneByteExtensions() &&
-                                               extenLen + 5 <= RTC::Consts::OneByteRtpExtensionMaxLength) ||
-                                         (packet->HasTwoBytesExtensions() &&
-                                               extenLen + 5 <= RTC::Consts::TwoBytesRtpExtensionMaxLength)
-                                       )
-                                       // clang-format on
-                                       {
-                                               extenLen += 5;
-                                       }
-                                       else
-                                       {
-                                               MS_WARN_DEV("cannot increase DD extension header length, current length %zu", extenLen);
-                                       }
-
                                        extensions.emplace_back(
                                          static_cast<uint8_t>(RTC::RtpHeaderExtensionUri::Type::DEPENDENCY_DESCRIPTOR),
                                          extenLen,

Now I have several important questions I need to answer myself after all, because we need a proper summary of the current state:

1. Do the freezes happen upon packet loss when DD extension is recvonly? This is crucial to know as it would completely turn the ongoing investigation. At this point I'm not completely sure of this, and must be assured.
2. Do the rendering work with pipe transports if the patch above is applied? If so, then there is obviously a problem with the packet rewrite. If not, then there is a more deep problem as we are merely passing the packet down the pipe as with the rest of codecs.
3. LiveKit apparently does not have the freezes when negotiating AV1 + DD and applying downlink packet loss. I've used this link few times and the number of freezes was lower, but this needs to be revisited. We need to know which scalability mode is used there, as we know that L1T3 also works in mediasoup.

Now, we need to implement pipe transport usage in mediasoup-demo in order to test this and pipe transports in general, in order to be able to test this reliably. Believe it or not, up to this date, there is no such implementation in the demo.

[ibc]

I will implement pipe transport in the new demo server next week. Said that, what about moving your previous comment into a new separate issue @jmillan ?

[vpalmisano]

3. LiveKit apparently does not have the freezes when negotiating AV1 + DD and applying downlink packet loss. I've used this link few times and the number of freezes was lower, but this needs to be revisited. We need to know which scalability mode is used there, as we know that L1T3 also works in mediasoup.

They use "L3T3_KEY":

[
    {
        "active": true,
        "adaptivePtime": false,
        "maxBitrate": 1190000,
        "maxFramerate": 30,
        "networkPriority": "low",
        "priority": "low",
        "scalabilityMode": "L3T3_KEY"
    }
]

[vpalmisano]

I will implement pipe transport in the new demo server next week. Said that, what about moving your previous comment into a new separate issue @jmillan ?

I'm running my test with these changes versatica/mediasoup-demo#149

[jmillan]

NOTE: I'll create a proper issue with all this info.

1. Do the freezes happen upon packet loss when DD extension is recvonly? This is crucial to know as it would completely turn the ongoing investigation. At this point I'm not completely sure of this, and must be assured.

No, setting the DD extension to recvonly there are virtually no freezes (freezeCount, totalFreezesDuration) compared to using sendrecv.

[jmillan]

NOTE: There are as much freezes if we modify DD (to add the Active Dependency Targets bitmask) or if we don't touch it, by applying this patch.

[jmillan]

So as for now we have these important points:

• There is not known implementation working with AV1 S3T3. mediasoup and google meet work with S1T3. So maybe we are chasing ghosts here.
• We'll need to try S3T3_KEY, as at least we know livekit does use it.
• We need to investigate why pipe transports do not work when using sendrecv on DD.

[ibc]

• We'll need to try S3T3_KEY, as at least we know livekit does use it.

This can be tested already using the demo with with &webcamScalabilityMode=S3T3_KEY