VOGONS

Search

Common searches

Multithreaded video capturing

Topic actions

Reply 40 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

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. 😉

Reply 41 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

Finally, a _real_ crash-free version (at least for my end). =)

diff:

48c47
- SDL_Thread *video_thread;

---
+ SDL_Thread *video_thread=NULL;

87c86
-     bool        thread_running;

---
+     int        thread_running; 

346,404c355,411
-     videohandle->thread_running = false;
-     return rc;

+     return videohandle->thread_running=0;

643c664
-             capture.video.thread_running = false;

---
+             capture.video.thread_running = 0;

680,682c704,706
+         if (!capture.video.thread_running){

+                     capture.video.thread_running=1;

+                     video_thread = SDL_CreateThread(CAPTURE_VideoThread, (void*)&capture.video);}

---
-         if (!capture.video.thread_running)

-             video_thread = SDL_CreateThread(CAPTURE_VideoThread, (void*)&capture.video);

Attachments

  • Filename
    hardware.cpp
    File size
    30.33 KiB
    Downloads
    239 downloads
    File comment
    70 FPS, multithreaded video capturing, crash fixed (100% sure!)
    File license
    Fair use/fair dealing exception

Reply 42 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

Stupid but "green" hack : half FPS recording (still multithreaded).

$ diff hardware.cpp.old hardware.cpp -iebwB --suppress-common-lines
398a
    }
.
343a
    secframescount++;
    if(secframescount==4)
        secframescount=1;
    if(secframescount==1||secframescount==3){
.
238c
        AVIOUTd((Bit32u)(1000000 * capture.video.fps/2));              /* Rate: Rate/Scale == samples/second */
.
211c
        AVIOUTd((Bit32u)(1000000 / capture.video.fps/2));       /* Microseconds per frame */
.
42a
int secframescount=0;
.

I don't know what the effects are on things like frameskip but it works for me, as it avoids full queues (and most duplicate frames).

My previous patch (10 FPS recording), did not keep the video in sync with the audio once the crash problem was fixed (weird, I know).

Attachments

  • Filename
    hardware.cpp
    File size
    30.31 KiB
    Downloads
    233 downloads
    File license
    Fair use/fair dealing exception

Reply 43 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

BUG : not casting to int made very poor videos.

$ diff hardware.cpp.old hardware.cpp -iebwB --suppress-common-lines
239c
        AVIOUTd((Bit32u)(1000000 * (int)capture.video.fps/2));              /* Rate: Rate/Scale == samples/second */
.
212c
        AVIOUTd((Bit32u)(1000000 / (int)capture.video.fps/2));       /* Microseconds per frame */
.

Sorry!

Attachments

  • Filename
    hardware.cpp
    File size
    30.32 KiB
    Downloads
    232 downloads
    File license
    Fair use/fair dealing exception

Reply 44 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

14 FPS Version. Still no visual difference when compared to a 70 FPS movie.

I'll stop at this framerate though. There's not much point of going below that.

Attachments

  • Filename
    hardware.cpp
    File size
    30.31 KiB
    Downloads
    243 downloads
    File license
    Fair use/fair dealing exception

Reply 45 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

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..

Attachments

  • Filename
    hardware.cpp
    File size
    30.35 KiB
    Downloads
    152 downloads
    File license
    Fair use/fair dealing exception

Reply 46 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

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):

diff --git a/hardware.cpp b/hardware.cpp
@@ -361,7 +361,7 @@ int CAPTURE_VideoCompressFrame(video_capture_t *videohandle, video_chunk_t chunk
 	else codecFlags = 0;
 	if (!videohandle->codec->PrepareCompressFrame( codecFlags, videohandle->format, (char *)chunk.pal, videohandle->buf, videohandle->bufSize))
 	{
-		CAPTURE_VideoEvent(TRUE);
+		CAPTURE_VideoEvent(true);
 		return 1;
 	}
 
@@ -404,7 +404,7 @@ int CAPTURE_VideoCompressFrame(video_capture_t *videohandle, video_chunk_t chunk
 	}
 	int written = videohandle->codec->FinishCompressFrame();
 	if (written < 0) {
-		CAPTURE_VideoEvent(TRUE);
+		CAPTURE_VideoEvent(true);
 		return 1;
 	}
 	CAPTURE_AddAviChunk( "00dc", written, videohandle->buf, codecFlags & 1 ? 0x10 : 0x0);
@@ -423,26 +423,29 @@ int CAPTURE_VideoCompressFrame(video_capture_t *videohandle, video_chunk_t chunk
 }
 
 #if (C_THREADED_CAPTURE)
+video_capture_t *videohandle = NULL;
 
 int CAPTURE_VideoThread(void *videohandleptr) {
-	video_capture_t *videohandle=(video_capture_t*)videohandleptr;
+	videohandle=(video_capture_t*)videohandleptr;
 	videohandle->thread_running = true;
 	int rc = 0;
-
 	/* Process queue */
-	while (!videohandle->q.empty()) {
-//		LOG_MSG("queue size %d",videohandle->q.size());
-		/* Process a block and write it to disk */
-		if (!rc) rc = CAPTURE_VideoCompressFrame(videohandle,videohandle->q.front());
-		else CaptureState &= ~CAPTURE_VIDEO;
-		free(videohandle->q.front().videobuf);
-		free(videohandle->q.front().audiobuf);
-		free(videohandle->q.front().pal);
-		/* Delete chunk from queue */
-		videohandle->q.pop();
+	while (CaptureState & CAPTURE_VIDEO) {
+		while (!videohandle->q.empty()) {
+	//		LOG_MSG("queue size %d",videohandle->q.size());
+			/* Process a block and write it to disk */
+			if (!rc) rc = CAPTURE_VideoCompressFrame(videohandle,videohandle->q.front());
+			else CaptureState &= ~CAPTURE_VIDEO;
+			free(videohandle->q.front().videobuf);
+			free(videohandle->q.front().audiobuf);
+			free(videohandle->q.front().pal);
+			/* Delete chunk from queue */
+			videohandle->q.pop();
+		}
 	}
 
 	videohandle->thread_running = false;
+	videohandle = NULL;
 	return rc;
 }
…Show last 60 lines
 
@@ -615,7 +618,7 @@ skip_shot:
 		case 16:capture.video.format = ZMBV_FORMAT_16BPP;break;
 		case 32:capture.video.format = ZMBV_FORMAT_32BPP;break;
 		default:
-			CAPTURE_VideoEvent(TRUE);
+      CAPTURE_VideoEvent(true);
 			return;
 		}
 
@@ -623,27 +626,27 @@ skip_shot:
 		if (!capture.video.handle) {
 			capture.video.handle = OpenCaptureFile("Video",".avi");
 			if (!capture.video.handle) {
-				CAPTURE_VideoEvent(TRUE);
+				CAPTURE_VideoEvent(true);
 				return;
 			}
 			capture.video.codec = new VideoCodec();
 			if (!capture.video.codec) {
-				CAPTURE_VideoEvent(TRUE);
+				CAPTURE_VideoEvent(true);
 				return;
 			}
 			if (!capture.video.codec->SetupCompress( width, height)) {
-				CAPTURE_VideoEvent(TRUE);
+				CAPTURE_VideoEvent(true);
 				return;
 			}
 			capture.video.bufSize = capture.video.codec->NeededSize(width, height, capture.video.format);
 			capture.video.buf = malloc( capture.video.bufSize );
 			if (!capture.video.buf) {
-				CAPTURE_VideoEvent(TRUE);
+				CAPTURE_VideoEvent(true);
 				return;
 			}
 			capture.video.index = (Bit8u*)malloc( 16*4096 );
 			if (!capture.video.index) {
-				CAPTURE_VideoEvent(TRUE);
+				CAPTURE_VideoEvent(true);
 				return;
 			}
 			capture.video.indexsize = 16*4096;
@@ -697,11 +700,14 @@ skip_shot:
 		/* If queue exceeds size limit, wait for capture thread to empty queue */
 		if (capture.video.q.size()>MAX_QUEUE_SIZE) {
 			LOG_MSG("Writing video to disk. Please wait...");
-			SDL_WaitThread(video_thread,NULL);
+			while(capture.video.q.size()>MAX_QUEUE_SIZE) {
+				SDL_Delay(1);
+			}
+			//SDL_WaitThread(video_thread,NULL);
 		}
 
 		/* If thread is not already running, start it */
-		if (!capture.video.thread_running)
+		if (!videohandle)
 			video_thread = SDL_CreateThread(CAPTURE_VideoThread, (void*)&capture.video);
 
 #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.

Results : It's so fast and stable now that I was able to record a 1 hour long video at 70 FPS without any issue. 😀

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...

Attachments

  • Filename
    hardware.cpp
    File size
    28.81 KiB
    Downloads
    112 downloads
    File comment
    Full patch source file.
    File license
    Fair use/fair dealing exception

Reply 47 of 47, by xcomcmdr

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

Patch to avoid busy waiting from the thread when the queue is empty :

diff --git a/hardware.cpp b/hardware.cpp
index 4195862..517b247 100644
--- a/hardware.cpp
+++ b/hardware.cpp
@@ -424,29 +424,39 @@ int CAPTURE_VideoCompressFrame(video_capture_t *videohandle, video_chunk_t chunk
 
 #if (C_THREADED_CAPTURE)
 video_capture_t *videohandle = NULL;
+SDL_cond* non_empty_queue = SDL_CreateCond();
+SDL_mutex* dummy_mutex = SDL_CreateMutex();
 
 int CAPTURE_VideoThread(void *videohandleptr) {
 	videohandle=(video_capture_t*)videohandleptr;
 	videohandle->thread_running = true;
 	int rc = 0;
-	/* Process queue */
-	while (CaptureState & CAPTURE_VIDEO) {
-		while (!videohandle->q.empty()) {
-	//		LOG_MSG("queue size %d",videohandle->q.size());
-			/* Process a block and write it to disk */
-			if (!rc) rc = CAPTURE_VideoCompressFrame(videohandle,videohandle->q.front());
-			else CaptureState &= ~CAPTURE_VIDEO;
-			free(videohandle->q.front().videobuf);
-			free(videohandle->q.front().audiobuf);
-			free(videohandle->q.front().pal);
-			/* Delete chunk from queue */
-			videohandle->q.pop();
-		}
-	}
 
-	videohandle->thread_running = false;
-	videohandle = NULL;
-	return rc;
+  /* while we have to record */
+  while(CaptureState & CAPTURE_VIDEO) {
+
+    /* Process queue while it is not empty */
+    while (!videohandle->q.empty()) {
+      //LOG_MSG("queue size %d",videohandle->q.size());
+
+      /* Process a block and write it to disk */
+      if (!rc) rc = CAPTURE_VideoCompressFrame(videohandle,videohandle->q.front());
+      else CaptureState &= ~CAPTURE_VIDEO;
+      free(videohandle->q.front().videobuf);
+      free(videohandle->q.front().audiobuf);
+      free(videohandle->q.front().pal);
+
+      /* Delete chunk from queue */
+      videohandle->q.pop();
+    }
+
+    /* Sleep while the queue is empty */
+    SDL_CondWait(non_empty_queue, dummy_mutex);
+  }
+
+  videohandle->thread_running = false;
+  videohandle = NULL;
+  return rc;
 }
…Show last 12 lines
 #endif // C_THREADED_CAPTURE
@@ -697,6 +707,9 @@ skip_shot:
 		/* Push avi chunk to queue */
 		capture.video.q.push(chunk);
 
+    /* Wake up thread */
+    SDL_CondSignal(non_empty_queue);
+
 		/* If queue exceeds size limit, wait for capture thread to empty queue */
 		if (capture.video.q.size()>MAX_QUEUE_SIZE) {
 			LOG_MSG("Writing video to disk. Please wait...");

Seems to work a little more better (ie : less occasionnal lags). Needs more testing, but no crashes so far.

Might be not needed. Might be useful to someone else, however.

Until I have something valuable to add, I will not post here nor hack anymore on this.

Attachments

  • Filename
    hardware.cpp
    File size
    29.15 KiB
    Downloads
    111 downloads
    File comment
    sources
    File license
    Fair use/fair dealing exception
Go to top of page Go to top of page
Back to DOSBox Development
Close window
Close window