RetransmissionBuffer: Avoid needless allocations when storing and removing items

[jmillan]

• FillItem() is replaced with CreateItem(), which constructs the item in place instead of default constructing it and assigning its members afterwards.

• Clear() and RemoveOldest() no longer call Item::Reset() before deleting the item. ~Item() already destroys sharedPacket (which is what decreases the RTP packet shared pointer counter), so the call was redundant. Worse, Item::Reset() does not reset the item in place but replaces sharedPacket with a newly constructed one, allocating yet another shared state just to destroy it immediately after.

[ibc]

Do we need the Item::Reset() method or can we remove it?

[jmillan]

Do we need the Item::Reset() method or can we remove it?

We don't. Only used in tests, I already commented this in the past. I'll remove it here.

[ibc]

Do we need the Item::Reset() method or can we remove it?

We don't. Only used in tests, I already commented this in the past. I'll remove it here.

Does Reset() make sense in the tests? It's useful for something? If so we can make the method only public with #ifdef MS_TEST and private otherwise. There are some classes in SCTP and BWE with that trick in their .hpp.

[jmillan]

Does Reset() make sense in the tests? It's useful for something? If so we can make the method only public with #ifdef MS_TEST and private otherwise. There are some classes in SCTP and BWE with that trick in their .hpp.

It does not. There's no reason to test something that is never used.

[jmillan]

Indeed I though you were referring to SharedPacket::Reset() which is also not used anywhere but in tests.

[ibc]

Indeed I though you were referring to SharedPacket::Reset() which is also not used anywhere but in tests.

Ok some trick applies there.