8000 Fix issue with file://con, connected to an anonymous pipe, and short reads by joncampbell123 · Pull Request #3176 · Haivision/srt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix issue with file://con, connected to an anonymous pipe, and short reads #3176

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joncampbell123
Copy link

file://con is using a non-blocking read, which is fine, however the read() call can return less bytes than requested if not enough data is there.

Short reads can cause some number of zero padding bytes to appear in the SRT stream.

Use FIONREAD to know ahead of time whether there is at least "chunk" bytes available to make sure read() fills the expected number of bytes.

If ioctl(...FIONREAD...) fails, it's assumed to be a file handle, proceed normally.

…AD to make sure all the data requested is there to read first to prevent short reads that cause zero padding in the SRT stream
@ethouris
Copy link
Collaborator
ethouris commented Jun 6, 2025

Did you check if this works also when you are sending blocks by 564 bytes to be then packed into a packet?

Does the error occur also in case when the feeding application is sending a portion of 1316 bytes and less than that number is returned by Read()? If not, then the fix should be different - zero-filling is a problem of transferring data, not reading from the device.

@joncampbell123
Copy link
Author
joncampbell123 commented Jun 18, 2025

The application in question generates an MPEG transport stream real-time to an anonymous pipe. The srt-live-transmit program is run with the other end of the anonymous pipe connected to stdin and asked to send from file://con. I noticed that sometimes the call to read() gets less than the amount of data it expects, but instead of waiting for more, it just returns whatever partial data it received.

It's a non-blocking read because of the code farther up that uses fcntl() to make it non-blocking.

The code change keeps the non-blocking behavior but makes sure that there is enough data waiting before reading it to prevent partial reads.

@ethouris
Copy link
Collaborator

Ok, I understand; the only thing that worries me is that in case when you have "too little bytes waiting", you still have the read-ready state on the console, which means it will be spin-reading (without reading, just calling the checker function) until the number of ready bytes exceed chunk. That will likely work without problems in your case, but in general you don't have a guarantee that the fact that you have read less bytes than chunk is only a short-living temporary situation. Many use cases state that data are sent in portions, USUALLY 1316 bytes, but it may be also 564 bytes expected to be then sent as a single packet; those apps will be no longer work as expected after this change.

This can be however changed by an application option. You'd have to do several things for this; I hope not too troublesome:

In srt-live-transmit.cpp source file:

  1. Find parse_args function; the options are there. Add your option there, follow the example of g_statsout option. First the name (entries with multiple names just contain multiple alternative names, for example in your case it would be something like {"fc", "fullchunk"}), then the expected argument rule. ARG_NONE means that the option can be only present or not, which is what you need.
  2. Above you have a structure LiveTransmitConfig and you likely need to add a new field (so that this value is visible in main()).
  3. After calling this function in main() there are rewrites to some variables with transmit_ prefix - those are global variables, you should add a new one.

These transmit_ variables are declared in transmitbase.hpp file. Definition should be provided in transmitmedia.cpp file where the Read functions are located. When you have that, you can use the value of this variable in ConsoleSource::Read definition; you should "reject too small reads" only if this value is true.

@joncampbell123
Copy link
Author

Another option, at least on Linux and systems that provide it, is to use poll() to detect when the file descriptor is in the EOF state. Then for EOF it reads the rest of the data. https://www.man7.org/linux/man-pages/man2/poll.2.html

@ethouris
Copy link
Collaborator

EOF means that the connection has been shut down; in case of live transmission it doesn't matter. Surely you can change the application so that it doesn't pass the data down the pipeline until it has collected the exact number of chunk of the data (that is, once you read less, you read again, expecting the "remaining" to be filled, until the data is exactly the size of chunk).

This can work, too, but still the option is required because this application is not intended to normally work this way.

@joncampbell123
Copy link
Author
joncampbell123 commented Jun 25, 2025

EOF means that the connection has been shut down; in case of live transmission it doesn't matter. Surely you can change the application so that it doesn't pass the data down the pipeline until it has collected the exact number of chunk of the data (that is, once you read less, you read again, expecting the "remaining" to be filled, until the data is exactly the size of chunk).

This can work, too, but still the option is required because this application is not intended to normally work this way.

If you're going to accept data through STDIN or anonymous pipes, asking software to send exactly "chunk" number of bytes, or even know w 71E0 hat that exact number is, is ridiculous. I'll update this pull request to check for closure of the anonymous pipe as part of the ioctl() change, using poll() or select().

Just for reference, this is srt-live-transmit, run by our software. Our software creates an anonymous pipe, holds the write end, fork+execs to srt-live-transmit with STDIN (handle 0) connected to the read end.

Perhaps a simpler change is to just remove the fcntl() farther up and set "may block" to true, so that read() blocks until the number of bytes it expects are available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0