Well, with no more crashes and no more long full-queues-stops (frames waiting to be recorded >= 400), it's 99% fixed.
I'll keep frameskip for the little unexplained (CPU usage != 100% or even 80%) little stops that happen when playing and recording at the same time. Thanks. 😉
It was still crashing after >~ 7 minutes of video, and "using" HUGE amounts of virtual memory (up to 11 Gb or so, then system crash occured..).
Fixed version attached. No more virtual memory madness according to htop, and I was able to record a long video (17 minutes).
I never noticed this bug until now because I rarely make videos that are longer than ~5 minutes.
PS : 99% of this code is from kekko's awesome work. I just made very little modifications so it won't crash on my end, and record no more than 14 frames per second.
PPS : A longer video recording (32 minutes) crashed with glibc detecting "a double free or corruption (!prev"). Oh well, I can live with a 30 minutes recording limit..
As I use DOSBox video recording feature once a year I believed my last "fix" really "fixed" this. I guess not.
So, going back to kekko's latest version from this thread as a base, I finally fixed :
- the crashing issue
- the memory leak (about 10 Gb of virtual process size after a few seconds 😳 )
- the lagging (because a lot of threads were created each second)
Here is the diff (formated by git):
1diff --git a/hardware.cpp b/hardware.cpp 2@@ -361,7 +361,7 @@ int CAPTURE_VideoCompressFrame(video_capture_t *videohandle, video_chunk_t chunk 3 else codecFlags = 0; 4 if (!videohandle->codec->PrepareCompressFrame( codecFlags, videohandle->format, (char *)chunk.pal, videohandle->buf, videohandle->bufSize)) 5 { 6- CAPTURE_VideoEvent(TRUE); 7+ CAPTURE_VideoEvent(true); 8 return 1; 9 } 10 11@@ -404,7 +404,7 @@ int CAPTURE_VideoCompressFrame(video_capture_t *videohandle, video_chunk_t chunk 12 } 13 int written = videohandle->codec->FinishCompressFrame(); 14 if (written < 0) { 15- CAPTURE_VideoEvent(TRUE); 16+ CAPTURE_VideoEvent(true); 17 return 1; 18 } 19 CAPTURE_AddAviChunk( "00dc", written, videohandle->buf, codecFlags & 1 ? 0x10 : 0x0); 20@@ -423,26 +423,29 @@ int CAPTURE_VideoCompressFrame(video_capture_t *videohandle, video_chunk_t chunk 21 } 22 23 #if (C_THREADED_CAPTURE) 24+video_capture_t *videohandle = NULL; 25 26 int CAPTURE_VideoThread(void *videohandleptr) { 27- video_capture_t *videohandle=(video_capture_t*)videohandleptr; 28+ videohandle=(video_capture_t*)videohandleptr; 29 videohandle->thread_running = true; 30 int rc = 0; 31- 32 /* Process queue */ 33- while (!videohandle->q.empty()) { 34-// LOG_MSG("queue size %d",videohandle->q.size()); 35- /* Process a block and write it to disk */ 36- if (!rc) rc = CAPTURE_VideoCompressFrame(videohandle,videohandle->q.front()); 37- else CaptureState &= ~CAPTURE_VIDEO; 38- free(videohandle->q.front().videobuf); 39- free(videohandle->q.front().audiobuf); 40- free(videohandle->q.front().pal); 41- /* Delete chunk from queue */ 42- videohandle->q.pop(); 43+ while (CaptureState & CAPTURE_VIDEO) { 44+ while (!videohandle->q.empty()) { 45+ // LOG_MSG("queue size %d",videohandle->q.size()); 46+ /* Process a block and write it to disk */ 47+ if (!rc) rc = CAPTURE_VideoCompressFrame(videohandle,videohandle->q.front()); 48+ else CaptureState &= ~CAPTURE_VIDEO; 49+ free(videohandle->q.front().videobuf); 50+ free(videohandle->q.front().audiobuf); 51+ free(videohandle->q.front().pal); 52+ /* Delete chunk from queue */ 53+ videohandle->q.pop(); 54+ } 55 } 56 57 videohandle->thread_running = false; 58+ videohandle = NULL; 59 return rc; 60 }
…Show last 60 lines
61 62@@ -615,7 +618,7 @@ skip_shot: 63 case 16:capture.video.format = ZMBV_FORMAT_16BPP;break; 64 case 32:capture.video.format = ZMBV_FORMAT_32BPP;break; 65 default: 66- CAPTURE_VideoEvent(TRUE); 67+ CAPTURE_VideoEvent(true); 68 return; 69 } 70 71@@ -623,27 +626,27 @@ skip_shot: 72 if (!capture.video.handle) { 73 capture.video.handle = OpenCaptureFile("Video",".avi"); 74 if (!capture.video.handle) { 75- CAPTURE_VideoEvent(TRUE); 76+ CAPTURE_VideoEvent(true); 77 return; 78 } 79 capture.video.codec = new VideoCodec(); 80 if (!capture.video.codec) { 81- CAPTURE_VideoEvent(TRUE); 82+ CAPTURE_VideoEvent(true); 83 return; 84 } 85 if (!capture.video.codec->SetupCompress( width, height)) { 86- CAPTURE_VideoEvent(TRUE); 87+ CAPTURE_VideoEvent(true); 88 return; 89 } 90 capture.video.bufSize = capture.video.codec->NeededSize(width, height, capture.video.format); 91 capture.video.buf = malloc( capture.video.bufSize ); 92 if (!capture.video.buf) { 93- CAPTURE_VideoEvent(TRUE); 94+ CAPTURE_VideoEvent(true); 95 return; 96 } 97 capture.video.index = (Bit8u*)malloc( 16*4096 ); 98 if (!capture.video.index) { 99- CAPTURE_VideoEvent(TRUE); 100+ CAPTURE_VideoEvent(true); 101 return; 102 } 103 capture.video.indexsize = 16*4096; 104@@ -697,11 +700,14 @@ skip_shot: 105 /* If queue exceeds size limit, wait for capture thread to empty queue */ 106 if (capture.video.q.size()>MAX_QUEUE_SIZE) { 107 LOG_MSG("Writing video to disk. Please wait..."); 108- SDL_WaitThread(video_thread,NULL); 109+ while(capture.video.q.size()>MAX_QUEUE_SIZE) { 110+ SDL_Delay(1); 111+ } 112+ //SDL_WaitThread(video_thread,NULL); 113 } 114 115 /* If thread is not already running, start it */ 116- if (!capture.video.thread_running) 117+ if (!videohandle) 118 video_thread = SDL_CreateThread(CAPTURE_VideoThread, (void*)&capture.video); 119 120 #else
First, "TRUE" was undefined.
Then a lot of threads were created because the queue was empty, hence the thread was closed, after that the queue wasn't empty anymore, another thread was created which processed the queue, and so on...
This was the root of the lag, the memory leak, and the race condition leading to crashes.
The fix is essentially this : the "videohandle" variable becomes a global variable, the thread stays while (CaptureState & CAPTURE_VIDEO) is true, the thread is created only if the videohandle is NULL, and the rest are minor fixes of problems created because of those modifications.
So I'm posting the resulting code if it can be useful to someone else. I'm sorry about the whole noise (all my previous posts which were quite useless. I would not mind it at all if they were removed 😈 ), and when you see the solution it's quite ridiculous that I did not found it waaaaay earlier.
But I guess that threads are difficult to master...