ovidiu,
do you have any comments regarding adding of ice attributes (at least candidate) to sdp parser and struct sdp_payload_attr?
-- juha
Hello Juha,
The right place to add them would be sdp_stream_cell (those attributes are stream attributes) The only thing is how to organize them: - a list of all attributes - several lists based on candidate-type or component-id or something else. It all depends on what do we want to access.
-ovidiu
On Fri, Dec 28, 2012 at 11:07 AM, Juha Heinanen jh@tutpro.com wrote:
do you have any comments regarding adding of ice attributes (at least candidate) to sdp parser and struct sdp_payload_attr?
Ovidiu Sas writes:
The right place to add them would be sdp_stream_cell (those attributes are stream attributes)
ovidiu,
my goal is to be able to figure out in rtpproxy/force_rtp_proxy function if a=candidate relay attribute(s) should be added when the function alters ip address and port of a media stream.
the terms are a bit confusing here. ice rfc 5245 uses terms session (stuff that follows s= line) and media stream (stuff that follows m= line).
anyway, if media stream contains a a=candidate line with component-id 1 (= rtp) or 2 (= rtcp), i would like to add a corresponding a=candidate relay line.
for example, if media stream contains line
a=candidate:c0626422 1 UDP 2113932031 192.98.102.1 18380 typ host
i would like to add (for example) a line:
a=candidate:r6ba1155 1 UDP 16777215 192.98.102.10 51345 typ relay
where 192.98.102.10 and 50345 are ip and port of the media relay.
The only thing is how to organize them:
- a list of all attributes
- several lists based on candidate-type or component-id or something else.
It all depends on what do we want to access.
the only thing i care about a a=candidate line is if a line with given type (1 or 2) exists. i don't care how many such lines exists. the other thing is that i need to know a place in sdp where the new a=candidate line can be added (it can be anywhere among the a= lines of the media stream).
so i thought to add these two fields to typedef struct sdp_stream_cell:
char *first_rtp_candidate; /**< RFC5245: pointer to first rtp candidate (if any) */ char *first_rtpc_candidate; /**< RFC5245: pointer to first rtpc candidate (if any) */
does this sound reasonable to you?
-- juha
On Sat, Dec 29, 2012 at 2:51 AM, Juha Heinanen jh@tutpro.com wrote:
Ovidiu Sas writes:
The right place to add them would be sdp_stream_cell (those attributes are stream attributes)
ovidiu,
my goal is to be able to figure out in rtpproxy/force_rtp_proxy function if a=candidate relay attribute(s) should be added when the function alters ip address and port of a media stream.
the terms are a bit confusing here. ice rfc 5245 uses terms session (stuff that follows s= line) and media stream (stuff that follows m= line).
When the sdp is parsed: - for each s line an sdp_session_cell is created; - for each m line an sdp_stream_cell is created; - for each codec identified by a payload in m line an sdp_payload_attr is created.
If an SDP is received with two streams (one for audio and one for video), we will have two m lines and therefore two sdp_stream_cell(s). Each stream will have it's own ICE attributes.
anyway, if media stream contains a a=candidate line with component-id 1 (= rtp) or 2 (= rtcp), i would like to add a corresponding a=candidate relay line.
for example, if media stream contains line
a=candidate:c0626422 1 UDP 2113932031 192.98.102.1 18380 typ host
i would like to add (for example) a line:
a=candidate:r6ba1155 1 UDP 16777215 192.98.102.10 51345 typ relay
where 192.98.102.10 and 50345 are ip and port of the media relay.
The only thing is how to organize them:
- a list of all attributes
- several lists based on candidate-type or component-id or something else.
It all depends on what do we want to access.
the only thing i care about a a=candidate line is if a line with given type (1 or 2) exists. i don't care how many such lines exists. the other thing is that i need to know a place in sdp where the new a=candidate line can be added (it can be anywhere among the a= lines of the media stream).
so i thought to add these two fields to typedef struct sdp_stream_cell:
char *first_rtp_candidate; /**< RFC5245: pointer to first rtp candidate (if any) */ char *first_rtpc_candidate; /**< RFC5245: pointer to first rtpc candidate (if any) */does this sound reasonable to you?
I think it would make sense to create a list of ice attributes so we can add the new attribute to the proper location. Just like we add sdp_payload_attr to each stream we should add ice attributes to each stream. This will make the retrieval of each attribute fast and adding a new ice attribute in proper place will be easier (as we would have proper pointers to where to insert the lumps).
The ice attributes management should be very similar to sdp_payload_attr management.
typedef struct sdp_ice_attr { struct sdp_ice_attr *next; str foundation; str component_id; str transport; str connection_addr str port; str candidate_type; int candidateType; /* ICE_HOST/ICE_SRFLX/ICE_PRFLX/ICE_RELAY/ICE_UNKNOWN */ } sdp_ice_attr_t;
and inside the sdp_stream_cell we add: int ice_attrs_num; /**< number of ICE attrs inside a stream */ struct sdp_ice_attr **p_sdp_ice_attr; /**< fast access pointers to ice attrs */ struct sdp_ice_attr *ice_attr;
Let me know what do you think?
-ovidiu
Ovidiu Sas writes:
I think it would make sense to create a list of ice attributes so we can add the new attribute to the proper location. Just like we add sdp_payload_attr to each stream we should add ice attributes to each stream.
ovidiu,
i agree, but that would be huge overhead on top of what i need to add the relay candidate lines.
This will make the retrieval of each attribute fast and adding a new ice attribute in proper place will be easier (as we would have proper pointers to where to insert the lumps).
i'm not sure about the proper place. it looks to me that start of the a lines is not currently stored anywhere, because the prefixes, e.g., a=rtpmap:, are omitted.
The ice attributes management should be very similar to sdp_payload_attr management.
typedef struct sdp_ice_attr { struct sdp_ice_attr *next; str foundation; str component_id; str transport; str connection_addr str port; str candidate_type; int candidateType; /* ICE_HOST/ICE_SRFLX/ICE_PRFLX/ICE_RELAY/ICE_UNKNOWN */ } sdp_ice_attr_t;
and inside the sdp_stream_cell we add: int ice_attrs_num; /**< number of ICE attrs inside a stream */ struct sdp_ice_attr **p_sdp_ice_attr; /**< fast access pointers to ice attrs */ struct sdp_ice_attr *ice_attr;
Let me know what do you think?
what you suggest is what i thought too, but as i mentioned, it means lots of extra work over what i currently need. parsing of candidate lines is all but a trivial thing to do.
how about if i do what you suggest, but don't fill in anything else in struct sdp_ice_attr except foundation and component_id? if someone later needs the other field values, they can then add parsing of more fields.
regarding the pointer to the start of a=candidate line, it would be foundation.s - 12, right?
-- juha
On Sun, Dec 30, 2012 at 5:26 AM, Juha Heinanen jh@tutpro.com wrote:
Ovidiu Sas writes:
I think it would make sense to create a list of ice attributes so we can add the new attribute to the proper location. Just like we add sdp_payload_attr to each stream we should add ice attributes to each stream.
ovidiu,
i agree, but that would be huge overhead on top of what i need to add the relay candidate lines.
This will make the retrieval of each attribute fast and adding a new ice attribute in proper place will be easier (as we would have proper pointers to where to insert the lumps).
i'm not sure about the proper place. it looks to me that start of the a lines is not currently stored anywhere, because the prefixes, e.g., a=rtpmap:, are omitted.
The ice attributes management should be very similar to sdp_payload_attr management.
typedef struct sdp_ice_attr { struct sdp_ice_attr *next; str foundation; str component_id; str transport; str connection_addr str port; str candidate_type; int candidateType; /* ICE_HOST/ICE_SRFLX/ICE_PRFLX/ICE_RELAY/ICE_UNKNOWN */ } sdp_ice_attr_t;
and inside the sdp_stream_cell we add: int ice_attrs_num; /**< number of ICE attrs inside a stream */ struct sdp_ice_attr **p_sdp_ice_attr; /**< fast access pointers to ice attrs */ struct sdp_ice_attr *ice_attr;
Let me know what do you think?
what you suggest is what i thought too, but as i mentioned, it means lots of extra work over what i currently need. parsing of candidate lines is all but a trivial thing to do.
how about if i do what you suggest, but don't fill in anything else in struct sdp_ice_attr except foundation and component_id? if someone later needs the other field values, they can then add parsing of more fields.
Sure, go ahead with it. Later on we will fill in everything else.
regarding the pointer to the start of a=candidate line, it would be foundation.s - 12, right?
Yes.
-
Juha Heinanen -
Ovidiu Sas