What is the purpose of srtp_event_data_t->stream if srtp_stream_t is opaque?

[ibc]

The event emitter provides a single argument of type srtp_event_data_t which contains a srtp_stream_t member. Why? It is 100% useless given that it is an opaque struct:

typedef struct srtp_stream_ctx_t *srtp_stream_t;

and srtp_stream_ctx_t is defined in the private file include/srtp_priv.h. So being in an event callback function I cannot even get the SSRC value of the affected stream:

void on_srtp_event(srtp_event_data_t* data) {
    uint32_t ssrc = data->stream->ssrc;
}

=> it produces:

error: member access into incomplete type 'struct srtp_stream_ctx_t'
        uint32_t ssrc = data->stream->ssrc;
                                    ^
/usr/local/include/srtp/srtp.h:275:16: note: forward declaration of 'srtp_stream_ctx_t'
typedef struct srtp_stream_ctx_t *srtp_stream_t;

so what is the purpose of the stream field in the event data? is there any way to get the SSRC value of the affected stream?

[saghul]

It's defined on srtp_priv.h, so not totally opaque ;-) You can include that file and you have access to the structure members.

[ibc]

The doc clearly says that srtp_stream_t is an opaque field and thus I cannot rely (or should not rely) on a non public API file (srtp_priv.h).

[saghul]

You only live once :-) On Aug 22, 2014 1:51 PM, "Iñaki Baz Castillo" notifications@github.com wrote:

The doc clearly says that srtp_stream_t is an opaque field and thus I cannot rely (or should not rely) on a non public API file (srtp_priv.h).

— Reply to this email directly or view it on GitHub https://github.com/cisco/libsrtp/issues/67#issuecomment-53051537.

[jfigus]

My guess is this is just an oversight by the person that added the event handler API. There's probably no way to address this without significant changes to the API. I've shared my opinion on another thread recently, we may want to look at a major uplift to the API and bump the major from 1 to 2. This will break backwards compatibility, but it'll allow us to clean up these opacity issues.

[ibc]

Agreed.

[jfigus]

Closing issue due to inactivity

[ibc]

Inactivity? :) I think the bug was properly reported and no more action was needed (but just a proper fix/implementation). I expected this to be done in the announced future new API design.

[jfigus]

This issue is tagged as a "question", not a "bug". Since the most recent discussion was October 9, it looked inactive to me.

[pabuhler]

Looks like this was missed in the move to 2.0, this is a shame I think it will be a long time to 3.0 . One option would be make a new api in 2.1 and depreciate the old api, removing it in 3.0 The fix would be to swap the srtp_stream_t argument with uint32_t SSRC. I wounder how many users are actually using this API.

[ibc]

I wounder how many users are actually using this API.

Probably no one (other than for logging purposes) as such a current API is of very little use.

[pabuhler]

So then the question is if we can just change the srtp_stream_t argument to be a uintptr_t, and set it to the SSRC in version 2.1 Changing it to be uint32_t in 3.0 .

[ibc]

Same SSRC may happen many times in different SRTP contexts/sessions. I expect the associated session should be given within the callback.

[pabuhler]

in 2.0 the event data signature is

typedef struct srtp_event_data_t {
  srtp_t        session;  /**< The session in which the event happend. */
  srtp_stream_t stream;   /**< The stream in which the event happend.  */
  srtp_event_t  event;

so changing it to

typedef struct srtp_event_data_t {
  srtp_t        session;  /**< The session in which the event happend. */
  uintptr_t    ssrc;       /**< The stream in which the event happend.  */
  srtp_event_t  event;

should be ok, and then you can map it to you application, or?

[ibc]

You are right.

[pabuhler]

ok, then I will make the change and we can see if any one reviews it :)

[paulej]

I looked at the code. My question: what is the reason for changing "srtp_stream_t stream;" when "srtp_t session;" is also present? It's also a typedef in srtp.h, with the actual type defined in srtp_priv.h. What am I missing?

[ibc]

It happens that srtp_priv.h is supposed to be private API, and we cannot rely on it.

[pabuhler]

srtp_t session is still opaque but it is used as the first argument to a few of the srtp_xxxx functions. It is also possible for the receiver of the callback to map back to which session the event happened on. With srtp_stream_t it is not possible to use it for anything, not even finding out the SSRC that generated the event. the combination of srtp_t and SSRC should give the user enough context to be able to handle the event.