shaka-project / shaka-streamer

A simple config-file based approach to preparing streaming media, based on FFmpeg and Shaka Packager.
https://shaka-project.github.io/shaka-streamer/
Apache License 2.0
198 stars 62 forks source link

Add Windows support #8

Closed joeyparrish closed 3 years ago

joeyparrish commented 5 years ago

We don't yet support Windows, due to our use of os.mkfifo. But we could conceivably support Windows another way.

mariocynicys commented 3 years ago

I was working on this fork it's running fine on windows based on my own tests, but can't pass all the karma tests. tests with -stream_loop -1 causes ffmpeg to run forever and hit the response time limit. i can't get how it doesn't do the same with -stream_loop -1 on linux can i get some help.

joeyparrish commented 3 years ago

tests with -stream_loop -1 causes ffmpeg to run forever and hit the response time limit.

Well, -stream_loop -1 is meant to make ffmpeg run forever. From the ffmpeg docs:

   -stream_loop number (input)
      Set number of times input stream shall be looped. Loop 0 means no loop, loop -1 means infinite loop.

The Karma test should call /stop in run_end_to_end_tests.py to shut down the "infinite" live stream when the test is complete. This calls controller.stop() to shut down Shaka Streamer. The controller node calls node.stop() on each of the nodes it manages, which is meant to shut down the various processes. You may want to investigate there, but please read on first. I think this may be a result of some of the choices you made in the Windows fifo implementation.

I was working on this fork ... can i get some help.

I just reviewed the diff of your changes so far.

I like your approach of using CreateNamedPipe to create a handle to a named pipe in a Windows-specific way, but I don't at all understand how your thread is supposed to work. I don't understand why there are three pipe handles, nor do I understand logic like this:

  if not peek[0]:
    pipe1, pipe2, pipe3 = pipe3, pipe1, pipe2

I can see that you're swapping handles around. This would make sense to me with two handles (to sort out which is the reader process and which is the writer process), but with three, it's very confusing what is happening there.

This comment is also suspicious:

  # Second instance, could be for the main Packager or the dead Packager

What "dead Packager"? This doesn't make sense to me. Only one instance of Packager should be started per run of Streamer, and each run of Streamer gets a unique path name with a new UUID.

It seems like you have problem with the structure you've built, and that you have some hacks to try to work around this, but I suspect you have not yet found the root cause of the issue that led you to this three-pipes solution.

I think you're broadly on the right track (CreateNamedPipe()), but here are some things to think about:

  1. The thread pipes data from a writing process to a reading process, and each pipe should only ever have one of each. There's no obvious reason you should ever need three pipe handles.
  2. Don't think of the pipes as "this is the Packager" and "this is ffmpeg". There are many processes that can be connected with pipes in different configuration. Think of the processes as "this is the writer" and "this is the reader". (In fact, the live stream config may be triggering a different set of pipelines/processes, breaking some solution you arrived at by trial and error.)
  3. This is the most suspicious line in terms of hanging the tests: pipe1, pipe2, pipe3 = pipe3, pipe1, pipe2. This means you will only ever move data (using the original values of these) from pipe1 to pipe2, or from pipe3 to pipe1. If you truly need three pipes, how are these the only two directions that you would need to support? If you got this wrong, couldn't that lead to a hang if the data isn't moving?
  4. Are you sure you need a thread at all? Why don't the processes writing to and reading from the named pipe communicate directly with one another through the OS? If the thread is truly needed, please try to explain in docstring for mkWinfifo exactly why this is necessary (in contrast to Linux, where this moving of data between processes is done automatically in the OS). I don't remember enough about these low-level APIs in Windows to say for sure, so documenting this thoroughly would be very helpful for us as maintainers of the project.

I hope this helps! Thanks so much for working on this.

mariocynicys commented 3 years ago

Thanks i will try to look into the controller.stop issue soon.

so why 3 pipes ? first of all, windows named pipes are quite different from linux ones, there is no command line way to create a pipe, it must be done programmatically using win32api or at least thats what i have found a long research , And the pipe when created , the program who has created it is the server , and all data must pass through him or from him(using writeFile and readFile funcs), so we cant also create a program to create a pipe and just halt, we need this server to be awake all the time to move the data between the two clients connected to it, thats why i used it as a THREAD , it might be more performant if it is made as a PROCESS with other programming language other than python (maybe powershell) but i didnt try, when i check the task manager while running shaka-streamer it is ffmpeg the subprocess taking the biggest lot of the cpu, couple of threads wont affect the performance much.

Now as for the pipes, when a pipe is created in a program it needs a path which is prefixed with \\.\pipe\, the first pipe created for this path must determine the number of instances for this path, so other pipes can be created later on.

nMaxInstances

The maximum number of instances that can be created for this pipe. The first instance of the pipe can specify this value; the ?same number must be specified for other instances of the pipe. Acceptable values are in the range 1 through PIPE_UNLIMITED_INSTANCES (255).

If this parameter is PIPE_UNLIMITED_INSTANCES, the number of pipe instances that can be created is limited only by the availability of system resources. If nMaxInstances is greater than PIPE_UNLIMITED_INSTANCES, the return value is INVALID_HANDLE_VALUE and GetLastError returns ERROR_INVALID_PARAMETER.

While i was designing the mkfifo replacement, i was running the python implementation of the pipe on the command line and used 2 command line programs to test my implementation ( ffplay which was the consumer process in this case , and type which was the feeder process , type is just the same as cat in linux), i was typing a video file into ffplay it worked fine, at this time i was using this file, after finalizing it i went ahead and put it into shaka-streamer , here i found on my test that every time Packager would start before FFmpeg, only very few times the opposite happens , so for this file to work i needed to put a time.sleep(1) before node.start() in the controller_node so i leave a good time for the OS to start executing the FFmpeg process, i felt this approach a little untidy and some race conditions can still happen though - if the OS decides to keep the process unexecuted for a long time passing the 1s.

I tried then to work on a design that will allow any process to start first , and switch their pipe instances if they are connected the wrong way, you can find this implementation in this file, tested it on the command line using the same 2 tools ( ffplay and type), worked fine , tested it on shaka-streamer using my lovely video file, with vod configs, and , the pipe fails again :"D.

This time with a pretty weird error, the packager connects first like every time, but it doesnt connect to the input pipe instance only, for some reason packager connects to 2 pipe instances instead of one. if i try to make a pipe with 2 instances and connect packager to it, the first instance would be connected to packager and works fine , the second instance would be connected to the same packager ( not a different process , as it has the same PID ) but this instance would give an error upon using it for any operation , it actually also gives an error when running ConnectNamedPipe on the instance it is connected to.

note: if a pipe instance is already connected, running ConnectNamedPipe returns immediately with no error

Generally , a process connects to pipe instances in the order the instances were created, so the packager would connect to pipe1 and pipe2 if it is the first process to be executed , maybe if i have 3 pipe instance and some process is connected to pipe1 , packager would connect to pipe2 and pipe3 ,and always the second packager pipe instance is a dead one. after knowing this the hard way, i made a new implementation , this file, it tries to make ConnectNamedPipe and if it fails because of some faulty process is connected to it, it will disconnect it and reconnect again, this implementation worked fine on my tests most of the time, there is a race condition in it where ffmpeg might try to connect in the time where the 2 pipes are busy and the 2nd pipe has been not disconnected yet, this race condition will have a higher chance to happen if the cpu is overloaded with stuff and the processes are being suspended a lot, with a big input and pipeline config files, the race condition might actually happen quite a lot.

I had the 3 pipes idea from a long time but didn't like it either and didnt bother trying it, felt it's kind of over engineering things, the oldest approach would have been fine if processes run in order, the second would have been fine if packager doesnt do what it does.

I pushed to the fork the first approach but sadly i had to put the time.sleep() for that to work as expected. decided later to give the 3 pipes idea a shot, and it worked fine with no race conditions :D. By creating 3 pipe instances, there will be always enough instances for packager and ffmpeg to hook to, i connect them all, then find and put ffmpeg's process in pipe1 and packager process in pipe2 while pipe3 will carry the dead 2nd packager faulty connection.

i know things look like parkour in this line xD, but i hope it makes sense now

  if not peek[0]:
    pipe1, pipe2, pipe3 = pipe3, pipe1, pipe2

This is the most reliable design i can think of right now, if you have any ideas please let me know.

sorry for such a long explanation :o

i will open an issue with the packager guys soon to know what happens on windows and hopefully they might fix it, and we might revert to the simpler implementation of the pipe.

also i have left some description of the problem in these commits { https://github.com/meryacine/shaka-streamer/commit/3eee7977ea3b819e8ee9d5ad40e214fd935bbeb1, https://github.com/meryacine/shaka-streamer/commit/0e8be6e67dfb14d3afaddfaecdd1c1a20ca95b8c, https://github.com/meryacine/shaka-streamer/commit/2862c6b9ac9f25c5a466181f722b5c67900fd384, https://github.com/meryacine/shaka-streamer/commit/ac86b2feef7efddfae3c5a81d9e557f92f8fae8b }

TheModMaker commented 3 years ago

Why not just use differently-named named pipes? If you use \\.\pipe\pipeFFmpeg for FFmpeg and \\.\pipe\pipePackager for the Packager, then it doesn't matter which starts first, since they connect to different pipe instances.

The double packager connection may be because the Packager is opening the file twice, once to probe something and one to read the data. You could file a bug on the packager for this.

joeyparrish commented 3 years ago

Packager works with named pipes in Linux without opening them twice. Possibly there is some detection of pipes in Packager which is not working on Windows.

As for differently-named pipes, the way that pipes are created in Streamer now is without regard for who will use them. So that would require more than a drop-in replacement for os.mkfifo.

I think the best thing is to detect which handle is for the reader and which the writer, and swap them if needed. But you should look into solving the double-open issue in Packager instead of trying to work around it in Python, IMO. It seems complicated and confusing.

mariocynicys commented 3 years ago

Here was the bug causing the infinite looping, the tests now complete successfully on windows :D. i will keep up with the Packager team, in case things change for the double connection issue.

mariocynicys commented 3 years ago

kqyang said that the input is handled with the Windows file APIs and the bug isn't probably related to Shaka-Packager. (the issue) I think that jacob's suggestion is the best way now to handle this pipe issue. what do you think ?

joeyparrish commented 3 years ago

What I said before is that the way that pipes are created in Streamer now is without regard for who will use them. So naming the pipes differently (pipeFFmpeg, pipePackager) would require more than a drop-in replacement for os.mkfifo.

I won't say that it can't be changed, but the architecture is built on named pipes that are essentially interchangeable. The system just says "I need 10 pipes" and hands their names to the various processes. If you need to change that to something like "I need a pipe with this name", I would need to be convinced why that is necessary and worthwhile.

Also, a counterargument to hard-coded pipe names is that it prevents two simultaneous instances of Streamer.

mariocynicys commented 3 years ago

The pipe names will not be so hardcoded , possibly we can use the same mkwinfifo function to make 2 pipes with the generated uuid ,, one we append "F" to it so that's for ffmpeg and one we append "P" , but the name of the pipe being propagated in the streamer is the initial uuid ,, and when each node receives the pipe string ,, it appends the right letter "F" or "P" . this should create no conflict between 2 steamers instances running at the same time.

mariocynicys commented 3 years ago

I will do a more indepth re search to find if there is some way to make a system pipe in windows like the unix's fifo.

mariocynicys commented 3 years ago

i might not be able convince you , but I think this change is the good windows-type way to handle the pipes,,, while the "3 pipe instances" way works (actually passed 10x46 tests), but it seems kinda weird logic and relies on the current state of Packager, this is in the case of creating pipes programmatically. but I will look up if there is an easier way to do so ,that can create an interchangeable pipe for both processes.

joeyparrish commented 3 years ago

The pipe names will not be so hardcoded ... one we append "F" to it so that's for ffmpeg and one we append "P"

That sounds reasonable to me, but I would again encourage you to get away from thinking of one end as ffmpeg and one as packager. There are other pipes than this one pairing, depending on configuration, and those will likely expand. Instead, you could use "W" for write and "R" for read. Every pipe will have one reader and one writer, so this is always a reasonable set of names.

aradalvand commented 3 years ago

Hey there! I've been using Shaka Packager + FFmpeg for some time now, and I watched Joey's presentation video on YouTube about Shaka Streamer a couple of months ago, and I was quite thrilled to find out that a tool like this exists to simplify this very process of using FFmpeg + Shaka Packager in such an elegant way. I'm a Windows user though, so it's quite a bummer not to be able to use Shaka Streamer on Windows :(

I know that this doesn't feel like the most pressing issue to those of us who use macOS or Linux on a regular basis, but the rest of us are feeling really deprived! 😅 So, any ideas on approximately when we can expect to have a Windows-compatible version of Shaka Streamer? Because that'd be truly fantastic. It's now been about 1 year and 10 months since this issue was created.

Thank you all for your efforts, by the way. Greatly appreciated!

joeyparrish commented 3 years ago

@meryacine and I hope to have a solution for Windows by mid-July.

There may be some work required in Shaka Packager to make it work correctly. In short, Packager does some odd things when we tried feeding it named pipes on Windows.

Other than that open issue, @meryacine has a branch for Windows support: https://github.com/meryacine/shaka-streamer/tree/windows