-
Notifications
You must be signed in to change notification settings - Fork 272
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
Fix for crash on seek #1469
base: master
Are you sure you want to change the base?
Fix for crash on seek #1469
Conversation
Second commit seems to allow the other two threads to cleanly end. Still not sure if this is the correct solution. I also suspect this case will never happen
|
Thinking about this code a bit more, if feels like it is pointless to keep in, also move the eof=true into the catch. Let me know if this should be squashed into 1 commit.
|
Looking at #1460 again... This would prevent the crash but would also end the video playback. Adding a new try catch in the loop would prevent that, the question is whether audio sync would then go out. V3.3 doesn't crash but won't play this file. |
Partial fix for CasparCG#1451 and CasparCG#1460
This seems to allow the Audio and Main thread to complete for the ffmpeg producer.
#1444 is also solved by this. Would be great for @ronag to give some feedback here. #1444 #1451 #1460 are all related to the changes in commit b657428. Before that commit there was a single thread where any exception would kill the pipeline and be caught at the .run call here. commit b657428 Now spawns 2 additional worker threads, one for video and another for audio and I'm not entirely sure how to stop one of the workers if the other throws other than setting the eof flag. For the seek issue we would want play to continue theoretically however for partial files we would want the threads to stop processing more data. With the try catch in the loop the video thread will stop but audio continues until a natural EOF is reached if ever, this could just end up stuck in a loop infinitely. But if we catch outside the loop seeking doesn't work. If we want to just error on a file that is broken and not attempt to recover then breaking outside the loop is fine. I feel likely 5033b57 is the way to go here |
Should we be catching |
Hi @jpc0 thank you for looking into this. |
https://github.com/CasparCG/server/commit/4f37f1be63704d24c69a3f87842f7be7c33086d0 I rewrote history when my fix for VS2022 got pulled. I left the two in history here though because I need maintainers to know which direction to go in. The issue with continueing on error is that the loop hangs indefinitely. So will likely need to pull some deeper errors out of FFMPEG and catch two seperate errors. |
Partial fix for #1451 and #1460
It is ugly but it does fix the crashing for the time being, however the audio thread keeps running as well as the main ffmpeg_producer thread.
I'm not familiar with boost::thread or boost::exception so maybe someone know how kill the other two threads when this thread throws. There may be an architectural issue here that needs to be resolved.