Summary

Add marshaling for CompoundPackets.

Motivation

We need In order to support sending CompoundPackets in pion/webrtc#581.

What did you expect?

use uint32 is good for devices compatibility, like: arm32 x86
uint32 is big enough for store ntptime, see the rtcp protocol

we should check other vars in pion/rtp or pion/rtcp
for 32bits arch devices compatibility

Your environment.

• Version: 7ca6b6f

What did you do?

rrWithPadding := []byte{
	// v=2, p=1, count=1, RR, len=7
	0xa1, 0xc9, 0x0, 0x7,
	// ssrc=0x902f9e2e
	0x90, 0x2f, 0x9e, 0x2e,
	// ssrc=0xbc5e9a40
	0xbc, 0x5e, 0x9a, 0x40,
	// fracLost=0, totalLost=0
	0x0, 0x0, 0x0, 0x0,
	// lastSeq=0x46e1
	0x0, 0x0, 0x46, 0xe1,
	// jitter=273
	0x0, 0x0, 0x1, 0x11,
	// lsr=0x9f36432
	0x9, 0xf3, 0x64, 0x32,
	// delay=150137
	0x0, 0x2, 0x4a, 0x79,
}
pkts, _ := Unmarshal(rrWithPadding)
fmt.Println(pkts[0].Header().Padding)

What did you expect?

For it to return true, since the padding flag is set.

What happened?

It returns false, because we use a synthetic header after parsing, and this header always has the padding bit set to false.

This is fine for most cases, but it means that we can't implement the validity check that requires the first packet in a compound doesn't have padding.

Rapid Resynchronization Request ==> Rapid Resynchronisation Request

Summary

https://tools.ietf.org/html/draft-holmer-rmcat-transport-wide-cc-extensions-01#page-5

Motivation

make sfu support send-BWE transport-cc

Summary

We should support sending reduced-size RTCP packets (RFC 5506). This reduces the overhead for sending AVPF feedback by allowing packets to be sent without a reception report and CNAME.

Using rtcp-rsize must be negotiated out of band (via SDP) so maybe it should be configurable by a switch.

There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.

Error type: Cannot find preset's package (github>whitesource/merge-confidence:beta)

Your environment.

• Version: 54fe03a

This was originally reported in a bug report by @kixelated on the srtp repo. I'm creating this report to submit a patch against.

Summary

Add helper functions to make it easier to construct SourceDescription CNAME packets.

• Make easier to use constructor for SourceDescriptions
• Add helpers for choosing RFC 7022 CNAMEs

Motivation

Currently it's cumbersome to generate CNAME packets:

cname := &SourceDescription{
	Chunks: []SourceDescriptionChunk{{
		Source: 1234,
		Items: []SourceDescriptionItem{{
			Type: SDESCNAME,
			Text: "cname",
		}},
	}},
}

Because every CompoundPacket requires a SourceDescription with a CNAME, we should make creating these packets easy.

String methods are helpful in debugging.

Summary

Change the rtcp.Packet interface to allow CompoundPacket to implement it. Return rtcp.Packet from Unmarshal.

Motivation

In order to support reduced-size RTCP (#21) we need to allow unmarshaling packets that aren't CompoundPackets. Reduced size RTCP packets are not valid CompoundPackets (missing CNAME, etc) so the signature of Unmarshal must change to allow returning either single packets or CompoundPackets.

One way to support this is to make CompoundPacket a type of Packet.

This change will also help fix pion/webrtc#581 which involves sending CompoundPackets from the webrtc side.

Unmarshal should be the one true way to decode RTCP packets.

We also have Decoder which is intended for use with streaming APIs via io.Reader. However, because it's based on a stream instead of datagrams Decoder cannot support the new CompoundPacket type (is it a RR,SD,BYE or a CompoundPacket{RR,SD,BYE})?

Glancing at the godoc, it might seem like Decoder and Unmarshal are equivalent and interchangable but only one supports CompoundPacket. We should remove Decoder so new API users don't get confused and choose the wrong one.

Because of #126 we are going to need to do a /v2 break. Anyone have features/ideas/changes they would like to see?

@jech @adamroach @kevmo314 @boks1971 @mengelbart @cnderrauber @OrlandoCo @aler9 @spe-dev You all are active in this repo or use it extensively.

As introduced by Go 1.18

See: https://go.dev/security/fuzz/

Many WebRTC implementations negotiate this payload extension. It would be nice if we supported it, especially in service of pion/webrtc#610.

https://tools.ietf.org/html/rfc3611

This issue provides visibility into Renovate updates and their statuses. Learn more

This repository currently has no open or pending branches.

• Check this box to trigger a request for Renovate to run again on this repository

Reproduce:

1. run the sfu example
2. curl sdp
3. wait the logs.

The latest version is causing parse issues with packets coming from Chrome. It seems I misread the rtcp-rsize spec and I will have to back out some of the previous rtcp-rsize changes to fix.

What I've learned:

• WebRTC implements reduced-size RTCP (RFC 5506) along with regular RTCP (RFC 3350). Previously we only implemented RFC 3350. Many of the recent changes to this repo work toward supporting RFC 5506 (issue here).
• In WebRTC, rtcp-rsize can be enabled or disabled via a flag in the SDP. Strict validation checks must be disabled to support RFC 5506, but should still be available opt-in if users are operating in RFC 3350 mode.

Some terminology that tripped me up in the spec:

• Compound RTCP packets have a specific meaning in RFC 5506. Our code assumes that a compound packet could mean any set of two or more packets. However, a CompoundPacket MUST have an SR or RR followed by an SDES with a CNAME.
• In reduced-size RTCP mode it is possible to receive multiple RTCP packets in a single datagram that do not form a CompoundPacket.

This presents a problem:

To follow the terminology in the spec, a CompoundPacket MUST be a set of SR/RR + CNAME packets. However:

• The previous API was Unmarshal([]byte) (CompoundPacket, error). If a reduced-size RTCP packet is passed to Unmarshal the returned packet will not be a valid compound packet.
• The current API Unmarshal([]byte) (Packet, error) has the same problem. Packet can be a CompoundPacket or a single packet of another type, but not multiple packets that don't form a CompoundPacket. I've also discovered this API involves casting to a CompoundPacket pointer in a clunky way.

Proposed solution:

Have Unmarshal return a packet slice

func Unmarshal([]byte) ([]Packet, error)

Add a matching Marshal that takes a packet slice

func Marshal([]Packet) ([]byte, error)

Keep CompoundPacket for users that want to opt-in to RFC 3350 validation

packets, err := Unmarshal(data)
...
err := CompoundPacket(packets).Validate()
if err != nil {
  // bad packet!
}

Curious for everyone's feedback. Want to get this right so we don't have to change it again!

Clean up some rough edges of the API in advance of v2.0.

• remove rcp.Header returns (they're now included in the Packet)
• choose better names

SenderReport.Unmarshal: 👍 👍

	offset := srReportOffset
	for i := 0; i < int(h.Count); i++ {
		rrEnd := offset + receptionReportLength
		if rrEnd > len(packetBody) {
			return errPacketTooShort
		}
		rrBody := packetBody[offset : offset+receptionReportLength]
		offset = rrEnd

		var rr ReceptionReport
		if err := rr.Unmarshal(rrBody); err != nil {
			return err
		}
		r.Reports = append(r.Reports, rr)
	}

ReceiverReport.Unmarshal: 👍

	for i := rrReportOffset; i < len(rawPacket) && len(r.Reports) < int(h.Count); i += receptionReportLength {
		var rr ReceptionReport
		if err := rr.Unmarshal(rawPacket[i:]); err != nil {
			return err
		}
		r.Reports = append(r.Reports, rr)
	}

@adwpc Regarding: https://github.com/pion/rtcp/blob/master/transport_layer_cc_test.go#L597

// change last byte '0b00000001' to '0b00000000', make ci pass
0x20, 0x1, 0x94, 0x0,
// 0b00100000, 0b00000001, 0b10010100, 0b00000001,
// the 'Want []byte' came from chrome, and
// the padding byte is '0b00000001', but i think should be '0b00000000' when i read the RFC, what's wrong?
// webrtc code: https://webrtc.googlesource.com/src/webrtc/+/f54860e9ef0b68e182a01edc994626d21961bc4b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc

Even though the RFC says "zero padding", Chrome implements it as:

  if (padding_length > 0) {
    for (size_t i = 0; i < padding_length - 1; ++i) {
      packet[(*position)++] = 0;
    }
    packet[(*position)++] = padding_length;
  }

https://chromium.googlesource.com/external/webrtc/+/refs/heads/master/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc#662

That is, zero padding where the last octet is the length of the padding itself.

This is consistent with the usage of the padding bit in https://tools.ietf.org/html/rfc3550#section-6.4.1 where

If the padding bit is set, this individual RTCP packet contains
some additional padding octets at the end which are not part of
the control information but are included in the length field.  The
last octet of the padding is a count of how many padding octets
should be ignored, including itself (it will be a multiple of
four).

So if the RTCP packet has Padding = True, then last octet of the zero padding is the length of padding itself. Right now Pion sends invalid feedback packets if Padding is set to true, and Chrome is rejecting these packets based on the invalid padding.

As a workaround setting Padding = False will make Chrome accept the feedback reports since it is explictly set to allow all zero padding as backwards compatibility with "older" clients. (https://chromium.googlesource.com/external/webrtc/+/refs/heads/master/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc#475)

I have prepared a pull request #53

i'm writing a sfu demo, i want to send a pli to publisher when i received a pli from subscriber, the code is like this:

    go func() {
        for {
            rtcpPacket := <-videoTrack.RTCPPackets
            //right here, i found:  rtcpPacket.Header().Type != rtcp.TypePayloadSpecificFeedback and rtcpPacket.Header().Count != rtcp.FormatPLI
            //then i add some fix with rtcp package
            if rtcpPacket.Header().Type == rtcp.TypePayloadSpecificFeedback && rtcpPacket.Header().Count == rtcp.FormatPLI {
                fmt.Println("Receiver.SendRTCP(rtcpPacket)")
                Receiver.SendRTCP(rtcpPacket)
            }
        }
    }()

Thanks to Max Hawkins!