-
Notifications
You must be signed in to change notification settings - Fork 896
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
base: master
Are you sure you want to change the base?
Conversation
…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
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. |
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. |
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 This can be however changed by an application option. You'd have to do several things for this; I hope not too troublesome: In
These |
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 |
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 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. |
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.