Open sthilden opened 1 year ago
Hi @sthilden Thanks for raising an important question! Indeed, the SRT library accepts control packets both with or without zero padding. I am not sure what's the benefit of having the padding. This might be a legacy of the UDT library, that can be removed given existing receivers of control packets ignore this padding. Potentially those packets could be extended in the future to contain CIF. For example, the SHUTDOWN packet may be extended with the "reason for shutdown". The modified structure may be signaled using the "subtype" field.
Answering your question, I believe there should be no control information field (CIF) for Keep-Alive, Shutdown, AckAck, and, Peer Error packets. Obviously, there MAY be this zero padding, and the specification cannot enforce the opposite.
Note that Wireshark's SRT dissector currently expects zero padding and reports a warning if it is not there. That's because the dissector was derived from the UDT dissector with this parsing of zero padding. Something to fix as well.
Thank you for your answer, @maxsharabayko!
To ensure that other implementations are compatible with the existing versions of the open source SRT library, I think it would be beneficial to clarify in the specification that there may be padding.
At least to me, e.g. the sentence "Shutdown control packets do not contain Control Information Field (CIF)." suggests that including any CIF data for a shutdown packet would be invalid.
I suggest to include something in the specification to clarify this, either that
I am not sure what is the correct way to go, though. Please let me know your thoughts on this.
I am thinking your suggestion no. 2 might be better, to allow padding up to the MTU side. Possible future format extensions may be signaled using the "subtype" field.
there may be trailing padding / trailing data to any control packets, and this should be ignored
Reading the specification for control packets, I want to ask about some clarification regarding packets without control information field (CIF).
For the packet types Keep-Alive, Shutdown, and AckAck, this draft specifies that the "control packets do not contain Control Information Field (CIF)". For the Peer Error packet, this is not explicitly specified, but no Control Field Information is shown in the packet structure.
However, the Haivision SRT library does insert a 32-bit word of zeros as the CIF for all these packet types. Also, in the Haivision SRT Technical Overview document from 2018, it is specified that the packet types Keep-Alive, Shutdown, and AckAck contain a 32-bit word of zeros as the control information field (CIF).
Sending packets without any CIF padding to the Haivison SRT library seems to work just fine. The library accepts the packets with or without the zero padding, at least for Keep-Alive, Shutdown, and AckAck, which I have tested.
As the Haivision SRT library inserts a 32-bit word of zero padding, it means another SRT implementation must support that there could be padding present, and not discard packets with padding as an invalid packet.
So, I wonder about the following;