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

rtp argument of RTPRawPacket complicates things #16

Closed
DavidNemeskey opened this issue Aug 1, 2018 · 5 comments
Closed

rtp argument of RTPRawPacket complicates things #16

DavidNemeskey opened this issue Aug 1, 2018 · 5 comments

Comments

@DavidNemeskey
Copy link

I use JRTPLIB to parse RTP / RTCP packages, and I was wondering if the rtp argument of the RTPRawPacket contructor makes any sense. Normally, this is how I would go about parsing a packet:

RTPRawPacket raw_packet(data);
RTPPacket rtp_packet(raw_packet);
if (rtp_packet.GetCreationError()) {
    RTCPCompoundPacket rtcp_packet(raw_packet);
    ...
} else {
    ...
}

However, this code fails when data represents an RTCP packet: in order to parse raw_packet as an RTPPacket, I have to instantiate it with rtp=true; but then the creation of rtcp_packet will fail by default. So effectively if rtp_packet could not be created, I need to create another raw packet, this time with rtp=false, and call the RTCPCompoundPacket constructor on that.

Not only is the process more cumbersome that it should be, with parsing in mind, the parameter doesn't make much sense, because it forces me to make a blind guess about the contents of the packet.

@j0r1
Copy link
Owner

j0r1 commented Aug 3, 2018

Originally, in RFC 3550, there was a port for the RTP packets and a different port for the RTCP packets, and the flag indicates on which port this packet was received. Now, RTP and RTCP packets can optionally be multiplexed, and to figure out if it's an RTP or RTCP packet one needs to check the second byte: if its value lies between 200 and 204, it's considered to be an RTCP packet, otherwise an RTP packet.

Perhaps I should change that boolean to an enum which has 'RTP', 'RTCP' and 'Guess' options, where the first two cause the current behaviour and the third looks at that second byte?

@DavidNemeskey
Copy link
Author

DavidNemeskey commented Aug 3, 2018 via email

@j0r1
Copy link
Owner

j0r1 commented Aug 5, 2018

After some thought it seemed easier to just add a different constructor which omits that boolean parameter. I've added it in a branch rawpacketguess, does that work for you?

@DavidNemeskey
Copy link
Author

I've just checked it, works perfectly.

@j0r1
Copy link
Owner

j0r1 commented Oct 6, 2018

Ok great, thanks for testing. I've added it to master

@j0r1 j0r1 closed this as completed Oct 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants