[sr-dev] adding ice attributes to sdp parser

[Ovidiu Sas]

On Sun, Dec 30, 2012 at 5:26 AM, Juha Heinanen <jh at 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.