Re: [reSIProcate] Preserving order of attributes in sdp
Does anyone see any problems with this patch? The only issue I can see is
that we are maintaining two lists (one list and one HashMap) to store the
sdp attributes, so it takes more memory - I'm not really concerned about
this though.
If I don't hear any complaints I will apply it later this week.
Scott
-----Original Message-----
From: Björn Andersson [mailto:bjorn.v.andersson@xxxxxxxxxxxx]
Sent: February 13, 2008 10:50 AM
To: Scott Godin
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] Preserving order of attributes in sdp
Here is my patch to assure the ordering of atributes without changing
the SdpContents-interface:
--- SdpContents.hxx.orig 2008-02-12 10:23:33.000000000 +0100
+++ SdpContents.hxx 2008-02-13 13:30:52.000000000 +0100
@@ -32,6 +32,7 @@
void addAttribute(const Data& key, const Data& value = Data::Empty);
void clearAttribute(const Data& key);
private:
+ std::list<std::pair<Data, Data> > mAttributeList;
HashMap< Data, std::list<Data> > mAttributes;
};
--- SdpContents.cxx.orig 2008-02-12 10:23:36.000000000 +0100
+++ SdpContents.cxx 2008-02-13 15:07:57.000000000 +0100
@@ -58,7 +58,8 @@
}
AttributeHelper::AttributeHelper(const AttributeHelper& rhs)
- : mAttributes(rhs.mAttributes)
+ : mAttributeList(rhs.mAttributeList),
+ mAttributes(rhs.mAttributes)
{
}
@@ -71,6 +72,7 @@
{
if (this != &rhs)
{
+ mAttributeList = rhs.mAttributeList;
mAttributes = rhs.mAttributes;
}
return *this;
@@ -96,21 +98,16 @@
ostream&
AttributeHelper::encode(ostream& s) const
{
- for (HashMap< Data, list<Data> >::const_iterator i =
mAttributes.begin();
- i != mAttributes.end(); ++i)
+ for (std::list<std::pair<Data, Data> >::const_iterator i =
mAttributeList.begin();
+ i != mAttributeList.end(); ++i)
{
- for (list<Data>::const_iterator j = i->second.begin();
- j != i->second.end(); ++j)
+ s << "a=" << i->first;
+ if (!i->second.empty())
{
- s << "a="
- << i->first;
- if (!j->empty())
- {
- s << Symbols::COLON[0] << *j;
- }
- s << Symbols::CRLF;
+ s << Symbols::COLON[0] << i->second;
}
- }
+ s << Symbols::CRLF;
+ }
return s;
}
@@ -135,6 +132,7 @@
if(!pb.eof()) skipEol(pb);
+ mAttributeList.push_back(std::make_pair(key, value));
mAttributes[key].push_back(value);
}
}
@@ -142,12 +140,22 @@
void
AttributeHelper::addAttribute(const Data& key, const Data& value)
{
+ mAttributeList.push_back(std::make_pair(key, value));
mAttributes[key].push_back(value);
}
void
AttributeHelper::clearAttribute(const Data& key)
{
+ for (std::list<std::pair<Data, Data> >::iterator i =
mAttributeList.begin();
+ i != mAttributeList.end(); )
+ {
+ std::list<std::pair<Data, Data> >::iterator j = i++;
+ if (j->first == key)
+ {
+ mAttributeList.erase(j);
+ }
+ }
mAttributes.erase(key);
}
@@ -1789,13 +1797,13 @@
Codec::getName() const
{
return mName;
-};
+}
int
Codec::getRate() const
{
return mRate;
-};
+}
Codec::CodecMap& Codec::getStaticCodecs()
{
/Björn
Björn Andersson wrote:
> Yes, we need a list to support rfc3407 (are there others?). I also agree
> that it is a problem changing the interface.
> I'll have a go with the patch but without the interface changes.
>
> /Björn
>
> Scott Godin wrote:
>
>> You are not the first person to have this problem. : )
>>
>> We discussed in the past changing the AttributeHelper class to store in a
list, so that we can maintain attribute insert order. There are some
properties of the current implementation that we will need to duplicate (for
backwards compatibility), and these will end being less efficient - but I
don't see a way around this:
>>
>> 1. Currently attributes with like keys, are grouped together - and are
retrievable in a list via getValues() - if we want to maintain insert order
then we should no longer group attributes with like keys - but we should
maintain the getValues interface - this will mean walking the attribute
list, to form the value list to return.
>> 2. The exists() api will need to walk the entire list.
>> 3. Codec Format parameter attributes (a=fmtp:) and rtpmap are stored in
the Codec class - so we have less flexibility over their ordering - but this
should be ok.
>>
>> Here are some older emails on the topic for reference:
>> http://list.resiprocate.org/archive/resiprocate-devel/msg03325.html - I
don't really like the interface change in this patch.
>> http://list.resiprocate.org/archive/resiprocate-devel/msg02578.html
>>
>> Any thoughts?
>>
>> Scott
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx [mailto:resiprocate-
>>> devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Björn Andersson
>>> Sent: Monday, February 11, 2008 8:58 AM
>>> To: resiprocate-devel@xxxxxxxxxxxxxxx
>>> Subject: [reSIProcate] Preserving order of attributes in sdp
>>>
>>> I'm trying to add fax-capabilities according to RFC3407, but because of
>>> the map-implementation it is impossible to keep the order stated in the
>>> RFC:
>>>
>>> "The individual capability descriptions in a capability set can be
>>> provided contiguously or they can be scattered throughout the session
>>> description. The first capability description MUST, however, follow
>>> immediately after the sequence number."
>>>
>>> v=0
>>> o=4001 866981 866981 IN IP4 148.122.211.58
>>> s=-
>>> c=IN IP4 148.122.211.58
>>> t=0 0
>>> m=audio 10628 RTP/AVP 8 0
>>> a=rtpmap:8 PCMA/8000
>>> a=rtpmap:0 PCMU/8000
>>> a=ptime:20
>>> a=cpar: a=T38FaxVersion:0
>>> a=cpar: a=T38MaxBitRate:14400
>>> a=cpar: a=T38FaxRateManagement:transferredTCF
>>> a=cpar: a=T38FaxMaxBuffer:336
>>> a=cpar: a=T38FaxMaxDatagram:176
>>> a=cpar: a=T38FaxUdpEC:t38UDPRedundancy
>>> a=cdsc:1 image udptl t38
>>> a=sqn:0
>>>
>>> There is no way to get capacity description after the sequence number.
>>>
>>> best regards
>>> Björn A.
>>>
>>>
>>>
>>>
>>>
>>>
>>> --
>>> This communication is confidential and intended solely for the
>>> addressee(s). Any unauthorized review, use, disclosure or distribution
>>> is prohibited. If you believe this message has been sent to you in
>>> error, please notify the sender by replying to this transmission and
>>> delete the message without disclosing it. Thank you.
>>> E-mail including attachments is susceptible to data corruption,
>>> interruption, unauthorized amendment, tampering and viruses, and we
>>> only send and receive e-mails on the basis that we are not liable for
>>> any such corruption, interception, amendment, tampering or viruses or
>>> any consequences thereof.
>>>
>>> _______________________________________________
>>> resiprocate-devel mailing list
>>> resiprocate-devel@xxxxxxxxxxxxxxx
>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>
>>>
>>
>>
>
>
--
This communication is confidential and intended solely for the addressee(s).
Any unauthorized review, use, disclosure or distribution is prohibited. If
you believe this message has been sent to you in error, please notify the
sender by replying to this transmission and delete the message without
disclosing it. Thank you.
E-mail including attachments is susceptible to data corruption,
interruption, unauthorized amendment, tampering and viruses, and we only
send and receive e-mails on the basis that we are not liable for any such
corruption, interception, amendment, tampering or viruses or any
consequences thereof.