No consistency when use some srtp_* functions.

[bozho-2003]

When I use function srtp_add_stream to add a new stream, this function expects ssrc value in host order and by contrast, when I use srtp_remove_stream to remove an existent stream, this function expect ssrc value in network-byte order.
So, this issue happened.
The same thing happens when I want to update an existent stream using function srtp_update_stream.

[paulej]

It appears that some of the srtp_*() functions assume that parameters will present the SSRC in host order and some assume the SSRC will be in network byte order. I'd personally like to see all APIs use host order when the SSRC is passed in as a paramter.

[pengyanhai]

The inconsistency is not good. We had to do the byte order conversion before calling the srtp_remove_stream and srtp_update_stream. When you get it fixed, please announce it clearly in the release note, because this fix also requires the consumer to update their code.

[bozho-2003]

Hi,
I figured out two solutions.

1. Just like this (It is only an example):

Add a new function like this:
srtp_err_status_t
srtp_remove_stream(srtp_t session, uint32_t ssrc) {
return remove_stream_inner(session, htonl(ssrc));
}

Rename srtp_remove_stream as remove_stream_inner like this.
//This function will be used in libsrtp and won't be seen out of libsrtp.
srtp_err_status_t
remove_stream_inner(srtp_t session, uint32_t ssrc) {
..... //As before.
}

This method only changes some interface function and the modification is very small.

2. I will investigate all code and make a detailed plan to change the original code because these changes make a huge influence.

So could you tell me how you think about these methods and which one you prefer?

[pabuhler]

Commit #220 fixed the internal bug where the required byte order was not being used but it did not address the concerns for having inconsistencies in the API, ie srtp_remove_stream() still expects the SSRC in network byte order where as the srtp_add_stream() & srtp_update_stream() expect the policy to contain the SSRC in host byte order. the question is if we can, as @pengyanhai suggests, fix this by changing srtp_remove_stream() in up coming 2.1 minor release as longs as it is well documented.

[jesup]

I'd suggest considering changing the function name/signature when you correct this, as that will breakage due to API change -- either remove the old one (compiler will notify you to update), or leave/deprecate it and add a new function. Note as a shared library, it can be best to leave old API entrypoints so existing apps don't fail to load; there's less pressure to keep them in link-only libraries.

[pabuhler]

@jesup the best idea is probably to add a new function and depreciate the old. Since we are unsure of the most common usage of libsrtp it would be best to leave the old function in the library until next major release to avoid loading issues. But then need to think of a new but equivalent name for srtp_remove_stream() maybe srtp_delete_stream() ?

[thisisG]

After discussing this we want to wait for a major version.