Make TransportLayerCC header dynamic

[aler9]

Description

Header shouldn't be a property but a method, like the one of every other packet, otherwise it can't be updated automatically when a property changes.

Since the padding flag is now set automatically, a test unit which wrongly expects the padding flag from a packet with no padding has been changed.

[codecov[bot]]

Codecov Report

Merging #142 (e0df7f5) into master (677965a) will decrease coverage by 0.06%. The diff coverage is 78.57%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
- Coverage   75.81%   75.74%   -0.07%     
==========================================
  Files          21       21              
  Lines        2398     2404       +6     
==========================================
+ Hits         1818     1821       +3     
- Misses        482      485       +3     
  Partials       98       98              

Flag	Coverage Δ
go	75.74% <78.57%> (-0.07%)	:arrow_down:
wasm	75.74% <78.57%> (-0.07%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
transport_layer_cc.go	68.90% <78.57%> (+1.03%)	:arrow_up:
header.go	67.21% <0.00%> (-6.56%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[kevmo314]

Header shouldn't be a property but a method, like the one of every other packet, otherwise it can't be updated automatically when a property changes.

I'm not sure if this was the intent, many of the other structs seem to model the over the wire format instead of the most dynamic. So an invalid TWCC packet header can't be modeled with this change.

But @mengelbart would be a better decision making, I will defer my judgment to either way :)

[aler9]

@kevmo314 this library doesn't allow to model invalid packet headers in case of any other packet type. If modeling invalid packet headers is to become a library feature, it should be allowed with all the other packet types too.

Furthermore, i don't think that most users want to fill Header.Length manually, since it's not immediate in most cases.

The only information that gets dropped by this PR is the Padding flag, but actually a packet with the Padding flag set to true and no padding leads to an invalid Marshal() result, since RFC specifications for most RTCP packets explicitly say that the padding flag should be set only when padding is present.

[mengelbart]

Technically I agree with @aler9 because it is what all the other packet types implement, too. But unfortunately, this is a breaking change. See also this comment. I would like to see this merged once we prepare a v2 release of this package.

[aler9]

@mengelbart we should create a v2 branch in order to start developing all the features listed in #127. A major upgrade can't be done unless it is synced with a major upgrade of all pion libraries, since it has consequences on all the dependent libraries (starting from pion/webrtc), therefore we could prepare a branch that can be merged exactly when it's time to do so, exactly like we did with pion/rtp. What do you think?