rtp icon indicating copy to clipboard operation
rtp copied to clipboard

Always set Marker bit to false?

Open josharian opened this issue 5 years ago • 5 comments

Packetize always sets the Marker bit for the last payload in a sequence. I don't believe that that is semantically appropriate. The marker bit is defined to be application- (codec-)specific.

For example, for voice codecs, RFC 3551 section 4.1 says:

For applications which send either no packets or occasional comfort-noise packets during silence, the first packet of a talkspurt, that is, the first packet after a silence period during which packets have not been transmitted contiguously, SHOULD be distinguished by setting the marker bit in the RTP data header to one. The marker bit in all other packets is zero. The beginning of a talkspurt MAY be used to adjust the playout delay to reflect changing network delays. Applications without silence suppression MUST set the marker bit to zero.

If you feed a sequence of pre-constructed media samples in, as in https://github.com/Sean-Der/audio-spew/blob/687cf84003383d5a9ced1384264d9b99f669cfe5/main.go#L88-L100, every single packet gets the Marker bit set, which is definitely not right.

I suspect that the correct thing for this package to do is to never set the Marker bit, leaving it to the parent application to set it as needed.

cc @Sean-Der

josharian avatar May 25 '20 23:05 josharian

This is a tough one, I am not sure how to solve it.

I think the responsibility should lie with the Payloader. Instead of returning a [][]byte maybe it should return a []Packetized

Then Packetized would be {[]byte, markerBit}? That would give all the codecs the ability to make as they please.

@at-wat What do you think? Also try to solve https://github.com/pion/rtp/issues/70

Sean-Der avatar Oct 24 '25 19:10 Sean-Der

Maybe we could try solving this with a With..?

I will try it a few different ways and see what feels best.

Sean-Der avatar Oct 24 '25 19:10 Sean-Der

Maybe we could try solving this with a With..?

@Sean-Der I had this idea when i was fixing #111 https://github.com/pion/rtp/pull/305 to fix this too, with an option.

But i don't remember why i actually didn't commit to it 🤔

JoTurk avatar Oct 24 '25 19:10 JoTurk

Packetizer option sets packetizer global option, but codec specific marker bit should be set for each packet. So, I don't think packetizer option cleanly and generally implement this. (Disabling marker bit of all packets can be implemented, but can't implement codec specific marker bit logic)

at-wat avatar Oct 28 '25 03:10 at-wat

I thought about implementing it like a filter WithMarkerBitFilter((packet byte[]) => bool), but thinking about it now it feels like a bad hack 🤔

JoTurk avatar Oct 28 '25 04:10 JoTurk