Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

srt-live-transmit default chunk size #702

Closed
maxsharabayko opened this issue May 24, 2019 · 2 comments · Fixed by #870
Closed

srt-live-transmit default chunk size #702

maxsharabayko opened this issue May 24, 2019 · 2 comments · Fixed by #870
Labels
[apps] Area: Test applications related improvements Type: Maintenance Work required to maintain or clean up the code
Milestone

Comments

@maxsharabayko
Copy link
Collaborator

At the moment srt-live-transmit has two parameters about the packets payload to configure.
URI socket option payloadsize and command-line argument -chunk (-c) that sets a size of a chunk. This chunk size determines the size of the buffer to store data read from source and pass to destination.
Both of these options are 1316 by default. But they both have to be configured to change the packet payload. If the payloadsize is increased, the chunk size stays the same and prevents having the full payload being read from source.
This provides inconvenience and complications, e.g. when trying to transmit RTP packets with srt-live-transmit over SRT.
In general, the chunk size option is redundant, except for the cases when stdin or file input are used as the source (the latter one if unavailable in srt-live-transmit).

Potential improvement of this could be to set the chunk size to 1500 bytes, unless file:con is selected as a source.
Or, maybe better, to get the payload size from the source and use it as a chunk size, thus removing -chunk command line argument.

@maxsharabayko maxsharabayko added Type: Enhancement Indicates new feature requests [apps] Area: Test applications related improvements labels May 24, 2019
@maxsharabayko maxsharabayko added this to the v.1.3.3 milestone May 24, 2019
@jeandube
Copy link
Collaborator

Don`t forget the MSS which is the Max Segment Size. Based on the network capability (MTU), its role was to prevent segmentation by IP, with default of 1500. SRT used that default until 1.2 at the cost of useless memory allocated in send and received buffers and a ACK sent for each packet (an incomplete packet being considered a end-of-file) which was disabled for SRT-CC then the equivalent of the actual live 'smoother'. The reduced payload size (and MSS) has raised many issues since its introduction to save memory nobody seems to care about.
I am not sure what problem setting the payload size instead of MSS solves. Beside being more convenient than full packet size since the protocol header sizes are not readily available above the API. I understand that according MSS to the maximum packet size MPEG-TS need (7 ts-cell x 188 bytes/cell) saves memory in the send and receive buffers.
The MSS is exchanged between the peers in the handshake and the max of the 2 is used by both sides.

@maxsharabayko maxsharabayko modified the milestones: v.1.3.3, v.1.3.4 May 29, 2019
@maxsharabayko maxsharabayko modified the milestones: v1.3.4, v1.4.0 Aug 19, 2019
@ethouris
Copy link
Collaborator

I have to document it somewhere. AFAIR the SRTO_PAYLOADSIZE option one use was to set the initial value of the payload that was needed for stats and bandwidth calculation. If it starts from 0, it will slowly reach up to the real average in the beginning (its evaluated through IIR), which makes the bandwidth calculated incorrectly for initial period. Simultaneously it limits the size of the data to be pushed to sending in a sending function and the minimum buffer size in the receiving function. This option can be as well set to 0 (usual case for file mode) and in this case simply restrictions won't apply, as well as it's not required for proper calculation in FileCC because this happens different way there.

Although I remember what you said about that and it's reasonable that when changing the value of SRTO_PAYLOADSIZE to a nonzero value, the synchronizing change to SRTO_MSS option should follow, as without this synchronization there's a wasted buffer space for single packets (another PR might be required). This change might be not so easy, though - a theoretical possibility that a larger packet comes in than it can fit in the buffer must be handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[apps] Area: Test applications related improvements Type: Maintenance Work required to maintain or clean up the code
Projects
None yet
3 participants