ShiftMediaProject / FFmpeg

Unofficial FFmpeg with added custom native Visual Studio project build tools. FFmpeg: A complete, cross-platform solution to record, convert and stream audio and video.
http://ffmpeg.org
Other
751 stars 290 forks source link

Udp complains about missing pthreads #18

Open ashikns opened 7 years ago

ashikns commented 7 years ago

Using the default project configuration, udp (for example when streaming over rtp) complains that pthreads support is required for circular buffers. Can this be solved with any config option? Or do I have to include pthread-win32 as a dependency?

Sibras commented 7 years ago

Exactly what are you doing when you get this "complaint". What is generating the complaint? Ill need complete bug information in order to resolve.

ashikns commented 7 years ago

I am streaming video and audio over rtsp. This works fine with the FFmpeg builds from zeranoe. However, when I build it on MSVC I get this error:

'circular_buffer_size' option was set but it is not supported on this build (pthread support is required)

You can find the relevant source code at https://ffmpeg.org/doxygen/trunk/udp_8c-source.html, line 557. Note that this is not a problem with ShiftMediaProject itself, I had faced this before when I built using MSVC with MSys2. What I'm asking is whether it would be possible to fix this in ShiftMediaProject.

Screenshot of issue: http://i.imgur.com/vYKMVmb.png

Sibras commented 7 years ago

OK, the additional info makes it much clearer. Currently only using pthreads will enable the missing functionality of the udp code. However there have been issues with pthreads-win32 reported before so i wouldnt recommend using them and that is why all provided projects use win32 threads.

Instead I have a patch that i quickly wrote up to see if it fixes the problem (i.e. enables the functionality without needing pthreads). In order to test to see if it fixes your use case you have to apply the patches and test them yourself. They are linked below so if you can test them out and ensure that it works correctly i will add it the upstream ffmpeg repo myself. Just "git am < [patch]' onto an existing ffmpeg repo to test them.

https://gist.github.com/Sibras/08fc183f280eb6451c736ce42182963b https://gist.github.com/Sibras/809de59dfbf0b8229c0b28c8b1e409ed

ashikns commented 7 years ago

Thanks! I am away from my main work machine right now, I'll try it out tomorrow and let you know how it goes :)

ashikns commented 7 years ago

I can confirm that the two patches solve the problem with udp, as in they don't complain anymore. However I am not sure of the implications on functionality, since streaming worked fine for me before and after applying the patches.

Sibras commented 7 years ago

udp would still work without the patches, however using the patch enables using a multithreaded packet queue. The previous errors you got was because you were setting certain settings that only applied to this packet queue (in this case the buffer size setting) hence why you got the error. The patch allows the multithreaded functionality when using win32 threads instead of the previous pthread requirement. So the main issue with this patch is to ensure all packets are still being received and are correctly decoded. I have updated the first gist as there was a logic bug in it. But if you can confirm the above requirements with the new patch then i will submit it upstream.

ashikns commented 7 years ago

I won't be able to test the updated gist until tomorrow. But other than that, yes I can confirm that this does indeed work. I've been using it for rtsp streaming and so far haven't faced any issues. Many thanks for your prompt response and help in this matter 👍

Sibras commented 7 years ago

OK the previous code patches i supplied were not fully thread safe so some improved versions are now available. These patches are pending inclusion in the main ffmpeg repo so it would be really helpfully if you could test these out and make sure they work for you.

https://gist.github.com/Sibras/6792316413f3a595d8a941d924cbe653 https://gist.github.com/Sibras/4c96b62db5f382ba004c56914a09f3fe https://gist.github.com/Sibras/6fef0e4a57e44576ed0929c913c7e9d5

If you could test these on a fresh ffmpeg repo and ensure udp streaming works correctly without errors (and also check that the additional udp thread is correctly shutdown) then i will add this to upstream straight away. Of course if you have some specific test cases you want to post (i.e. exact command lines) then i will test them myself as well.

ashikns commented 7 years ago

I'm currently held up with some other stuff, so it'll be a while before I can follow up on this. I'm still using the FFmpeg build with your original patches and it works fine, but that's all I can provide for now. Sorry :(