Windows deprecated stdin reading method
So trying to compile on VS2015 now works fine for the libraries. However the command-line tool does not compile successfully. The issue is this patch https://github.com/FFmpeg/FFmpeg/commit/ca4d71b149ebe32aeaf617ffccf362624b9a... which uses a member of the FILE struct which is not available in the new C runtime (it's now just a struct with a void*). I do not quite know the semantics of the stdin->_cnt > 0 check. So I tried a few approaches, like using the deprecated function that is called after the #ifdef block anyway: https://github.com/Bigpet/FFmpeg/commit/b167e17e6f8839e1b4ce0edad2708ff4237e... (I'm kind of worried about this since using "read(0, &ch, 1);" instead of "getch" with that if condition actually blocks in some cases, so it's not 100% equivalent) Or trying to understand what the non-deprecated Win32 API calls would look like: https://github.com/Bigpet/FFmpeg/commit/8d9cae13f389ab7dddc736ecac3feb01843c... But in the end I don't understand the purpose of the original piece of code, so it's hard to know what it should be replaced by. And finding documentation on it seems fruitless as well, since it used a non-public API to begin with. Any help from people knowledgeable about the windows API would be appreciated.
On 07.03.2015, at 16:51, Peter <dravorek@googlemail.com> wrote:
The issue is this patch https://github.com/FFmpeg/FFmpeg/commit/ca4d71b149ebe32aeaf617ffccf362624b9a... which uses a member of the FILE struct which is not available in the new C runtime (it's now just a struct with a void*).
I do not quite know the semantics of the stdin->_cnt > 0 check.
It checks if there is data in the buffer. The point is to do a non-blocking read on stdin.
So I tried a few approaches, like using the deprecated function that is called after the #ifdef block anyway: https://github.com/Bigpet/FFmpeg/commit/b167e17e6f8839e1b4ce0edad2708ff4237e... (I'm kind of worried about this since using "read(0, &ch, 1);" instead of "getch" with that if condition actually blocks in some cases, so it's not 100% equivalent)
Or trying to understand what the non-deprecated Win32 API calls would look like: https://github.com/Bigpet/FFmpeg/commit/8d9cae13f389ab7dddc736ecac3feb01843c...
I don't think that's generic enough, you can't know what kind of thing stdin is, so using any non-generic function on it might break things...
It checks if there is data in the buffer. The point is to do a non-blocking read on stdin.
I don't know how else to do that for stdin. Using WaitForSingleObject with generic functions like read() from io.h or ReadFile doesn't work since WaitForSingleObject will trigger on mouse and windows events, so calling those functions can result in blocking (even if I wait for an actual keyboard event). Or is that code specifically there to deal with everything that is not a console and what I actually need to do there is somehow typecheck that GetStdHandle did not return a console and only then call one of the generic read functions? (and let the kbhit and getch that come afterwards handle the console) 2015-03-07 23:38 GMT+01:00 Reimar Döffinger <Reimar.Doeffinger@gmx.de>:
On 07.03.2015, at 16:51, Peter <dravorek@googlemail.com> wrote:
The issue is this patch https://github.com/FFmpeg/FFmpeg/commit/ca4d71b149ebe32aeaf617ffccf362624b9a... which uses a member of the FILE struct which is not available in the new C runtime (it's now just a struct with a void*).
I do not quite know the semantics of the stdin->_cnt > 0 check.
It checks if there is data in the buffer. The point is to do a non-blocking read on stdin.
So I tried a few approaches, like using the deprecated function that is called after the #ifdef block anyway: https://github.com/Bigpet/FFmpeg/commit/b167e17e6f8839e1b4ce0edad2708ff4237e... (I'm kind of worried about this since using "read(0, &ch, 1);" instead of "getch" with that if condition actually blocks in some cases, so it's not 100% equivalent)
Or trying to understand what the non-deprecated Win32 API calls would look like: https://github.com/Bigpet/FFmpeg/commit/8d9cae13f389ab7dddc736ecac3feb01843c...
I don't think that's generic enough, you can't know what kind of thing stdin is, so using any non-generic function on it might break things... _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Alright, I may have been really stupid, for some reason I always passed "input_handle" to read and WaitForSingleObject instead of stdin, no clue what happened there in my head. Turns out this actually works in my initial tests: https://github.com/Bigpet/FFmpeg/commit/93864dd0373bc6561be8e45f14f835453c74... 2015-03-08 0:22 GMT+01:00 Peter <dravorek@googlemail.com>:
It checks if there is data in the buffer. The point is to do a non-blocking read on stdin.
I don't know how else to do that for stdin. Using WaitForSingleObject with generic functions like read() from io.h or ReadFile doesn't work since WaitForSingleObject will trigger on mouse and windows events, so calling those functions can result in blocking (even if I wait for an actual keyboard event).
Or is that code specifically there to deal with everything that is not a console and what I actually need to do there is somehow typecheck that GetStdHandle did not return a console and only then call one of the generic read functions? (and let the kbhit and getch that come afterwards handle the console)
2015-03-07 23:38 GMT+01:00 Reimar Döffinger <Reimar.Doeffinger@gmx.de>:
On 07.03.2015, at 16:51, Peter <dravorek@googlemail.com> wrote:
The issue is this patch https://github.com/FFmpeg/FFmpeg/commit/ca4d71b149ebe32aeaf617ffccf362624b9a... which uses a member of the FILE struct which is not available in the new C runtime (it's now just a struct with a void*).
I do not quite know the semantics of the stdin->_cnt > 0 check.
It checks if there is data in the buffer. The point is to do a non-blocking read on stdin.
So I tried a few approaches, like using the deprecated function that is called after the #ifdef block anyway: https://github.com/Bigpet/FFmpeg/commit/b167e17e6f8839e1b4ce0edad2708ff4237e... (I'm kind of worried about this since using "read(0, &ch, 1);" instead of "getch" with that if condition actually blocks in some cases, so it's not 100% equivalent)
Or trying to understand what the non-deprecated Win32 API calls would look like: https://github.com/Bigpet/FFmpeg/commit/8d9cae13f389ab7dddc736ecac3feb01843c...
I don't think that's generic enough, you can't know what kind of thing stdin is, so using any non-generic function on it might break things... _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 08.03.2015, at 00:46, Peter <dravorek@googlemail.com> wrote:
Alright, I may have been really stupid, for some reason I always passed "input_handle" to read and WaitForSingleObject instead of stdin, no clue what happened there in my head.
Turns out this actually works in my initial tests:
https://github.com/Bigpet/FFmpeg/commit/93864dd0373bc6561be8e45f14f835453c74...
It looks reasonable, but I'd expect there is a reason we didn't do this originally? Have you tested in the normal console, cygwin console, with stdin redirection etc?
On Sun, Mar 8, 2015 at 1:01 AM, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:
On 08.03.2015, at 00:46, Peter <dravorek@googlemail.com> wrote:
Alright, I may have been really stupid, for some reason I always passed "input_handle" to read and WaitForSingleObject instead of stdin, no clue what happened there in my head.
Turns out this actually works in my initial tests:
https://github.com/Bigpet/FFmpeg/commit/93864dd0373bc6561be8e45f14f835453c74...
It looks reasonable, but I'd expect there is a reason we didn't do this originally? Have you tested in the normal console, cygwin console, with stdin redirection etc?
Unfortunately, the original change does not really explain anything on the hows and whys. However, accessing FILE internals was always a terrible solution since its a private struct, so any improvements on that part are welcome, and from the documentation I read on the subject, this change seem sensible. - Hendrik
Sadly it seems WaitForSingleObject behavior with FILE handles is not well defined behavior either, I think I've read the words "strongly discouraged" I'm probably still gonna do some more tests to see if it actually breaks under some circumstances. But this is not the golden ticket solution either it seems. 2015-03-08 2:11 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
Unfortunately, the original change does not really explain anything on the hows and whys. However, accessing FILE internals was always a terrible solution since its a private struct, so any improvements on that part are welcome, and from the documentation I read on the subject, this change seem sensible.
- Hendrik _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
So testing on Windows 8.1 the results seem identical ||shell ||command ||stdin->_cnt > 0||WaitForSingleObject|| ||=================||=========================================||===============||===================|| ||cmd.exe || .\test.exe <a.txt || input ignored || input ignored || ||cmd.exe || type a.txt | .\test.exe || works || works || ||msys/cmd.exe || .\test.exe <a.txt || input ignored || input ignored || ||msys/cmd.exe || cat a.txt | .\test.exe || works || works || ||powershell || Get-Content input-file.txt | .\test.exe || works || works || ||cygwin/mintty.exe|| .\test.exe <a.txt || input ignored || input ignored || ||cygwin/mintty.exe|| cat a.txt | .\test.exe || works || works || I'm not sure I'm doing the redirection right. If there's something else that I need to test I would appreciate some input (no clue what can reasonably passed as stdin to CreateProcess or whether testing that is necessary) Also, very crucially, I just checked if any of these methods actually triggered either condition and none of the things I tried actually entered the body of the if-check. Is there anything that would normally trigger the "stdin->_cnt > 0" in that location ? 2015-03-08 2:25 GMT+01:00 Peter <dravorek@googlemail.com>:
Sadly it seems WaitForSingleObject behavior with FILE handles is not well defined behavior either, I think I've read the words "strongly discouraged"
I'm probably still gonna do some more tests to see if it actually breaks under some circumstances. But this is not the golden ticket solution either it seems.
alright, that table didn't work out, just imagine this: .\test.exe <a.txt input ignored type a.txt | .\test.exe works .\test.exe <a.txt input ignored cat a.txt | .\test.exe works Get-Content input-file.txt | .\test.exe works .\test.exe <a.txt input ignored cat a.txt | .\test.exe works across both approaches 2015-03-08 12:07 GMT+01:00 Peter <dravorek@googlemail.com>:
So testing on Windows 8.1 the results seem identical
||shell ||command ||stdin->_cnt > 0||WaitForSingleObject|| ||=================||=========================================||===============||===================|| ||cmd.exe || .\test.exe <a.txt || input ignored || input ignored || ||cmd.exe || type a.txt | .\test.exe || works || works || ||msys/cmd.exe || .\test.exe <a.txt || input ignored || input ignored || ||msys/cmd.exe || cat a.txt | .\test.exe || works || works || ||powershell || Get-Content input-file.txt | .\test.exe || works || works || ||cygwin/mintty.exe|| .\test.exe <a.txt || input ignored || input ignored || ||cygwin/mintty.exe|| cat a.txt | .\test.exe || works || works ||
I'm not sure I'm doing the redirection right. If there's something else that I need to test I would appreciate some input (no clue what can reasonably passed as stdin to CreateProcess or whether testing that is necessary)
Also, very crucially, I just checked if any of these methods actually triggered either condition and none of the things I tried actually entered the body of the if-check.
Is there anything that would normally trigger the "stdin->_cnt > 0" in that location ?
2015-03-08 2:25 GMT+01:00 Peter <dravorek@googlemail.com>:
Sadly it seems WaitForSingleObject behavior with FILE handles is not well defined behavior either, I think I've read the words "strongly discouraged"
I'm probably still gonna do some more tests to see if it actually breaks under some circumstances. But this is not the golden ticket solution either it seems.
On Sun, Mar 8, 2015 at 12:09 PM, Peter <dravorek@googlemail.com> wrote:
alright, that table didn't work out, just imagine this:
.\test.exe <a.txt input ignored type a.txt | .\test.exe works .\test.exe <a.txt input ignored cat a.txt | .\test.exe works Get-Content input-file.txt | .\test.exe works .\test.exe <a.txt input ignored cat a.txt | .\test.exe works
across both approaches
Did you ever make any further progress on this? Now that VS2015 is out and about, I would really love to get this fixed. Otherwise I'm just going to send a patch to remove the offending check, as it doesn't seem to trigger in common testing, and we can then see about fixing any fallout later. - Hendrik
participants (3)
-
Hendrik Leppkes -
Peter -
Reimar Döffinger