cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.19k stars 469 forks source link

fix srtp_remove_stream to use SSRC in host byte order #670

Closed pabuhler closed 6 months ago

pabuhler commented 6 months ago

This fixes the inconsistency in SSRC byte order usage in the public api. Now all usage should be in host byte order.

220

JonathanLennox commented 6 months ago

See @jesup 's comment on #220 at https://github.com/cisco/libsrtp/issues/220#issuecomment-274152974: I don't think changing this while leaving the function name and signature the same is a good idea. Even at an API-breaking change like 3.0.0, this won't have any compiler warning to tell someone porting to the new API that they need to change something.

pabuhler commented 6 months ago

@JonathanLennox fair point, so what about the new name, srtp_delete_stream() ?
Other options could be to change from add/remove to create/destroy ... Just changing the name though will not ensure people take notice of the change in parameters.

paulej commented 6 months ago

Maybe srtp_stream_add and srtp_stream_remove? Just changing the order, but I prefer to have the noun before the action in APIs. APIs are mixed in this regard, too.

pabuhler commented 6 months ago

Maybe srtp_stream_add and srtp_stream_remove? Just changing the order, but I prefer to have the noun before the action in APIs. APIs are mixed in this regard, too.

I think I like that, how about, I rename to srtp_stream_remove / add / update and add a note to the documentation of srtp_stream_remove about the change in byte order. It will also end up in the CHANGES file.

Can make a new task to ensure all API's are reviewed for naming consistency.

paulej commented 6 months ago

I think I like that, how about, I rename to srtp_stream_remove / add / update and add a note to the documentation of srtp_stream_remove about the change in byte order. It will also end up in the CHANGES file.

Can make a new task to ensure all API's are reviewed for naming consistency.

Sounds good to me.