From 5d86941b4ddc8cdde600edeb1e67b2cd85480975 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sat, 21 Oct 2023 23:55:18 -0700 Subject: get_frames,cache_frame: reset yuv blocks once cached The existing code was assuming every frame would get cached before the next frame was collected. But that's not strictly enforced in the current architecture. Sure, the condition variable is signaled for every frame, but that doesn't guarantee the cache_frame side will wake up and actually do its thing before get_frames moves on to the next frame. There's no waiting for cache_frame to ack the signal before grabbing the next frame. So in damage-tracking mode (the default when available), there was a very real potential for lost damage, especially with higher fps rates. This commit moves the resetting of the YUV dirty blocks to the cache_frame side, only after the accumulated dirty blocks have been processed, immediately prior to releasing yuv_mutex. As a side effect, the previously get_frames-private rmdBlocksReset() has moved to rmd_yuv_utils.c and renamed to rmdYuvBlocksReset(). Some comments have been thrown in flagging the need to refactor the YUV blocks creation/maintenance - it's all very ad-hoc as-inherited, I haven't cleaned that up, and it needs it badly. Hopefully this fixes some of the lost damage I've been noticing doing 60-fps captures of programming. --- src/rmd_cache_frame.c | 2 ++ src/rmd_get_frames.c | 11 ----------- src/rmd_initialize_data.c | 29 +++++++++++++++++------------ src/rmd_rescue.c | 29 +++++++++++++++++------------ src/rmd_yuv_utils.c | 13 +++++++++++++ src/rmd_yuv_utils.h | 3 +++ 6 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/rmd_cache_frame.c b/src/rmd_cache_frame.c index c37f530..be40872 100644 --- a/src/rmd_cache_frame.c +++ b/src/rmd_cache_frame.c @@ -219,6 +219,8 @@ void *rmdCacheImageBuffer(ProgData *pdata) 0); } + rmdYuvBlocksReset(pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH, + pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH); //release main buffer pthread_mutex_unlock(&pdata->yuv_mutex); diff --git a/src/rmd_get_frames.c b/src/rmd_get_frames.c index 2b442b5..3a9c2a3 100644 --- a/src/rmd_get_frames.c +++ b/src/rmd_get_frames.c @@ -210,14 +210,6 @@ static void rmdMoveCaptureArea( XRectangle *rect, } -static void rmdBlocksReset(unsigned int blocks_w, unsigned int blocks_h) -{ - memset(yblocks, 0, blocks_w * blocks_h * sizeof(*yblocks)); - memset(ublocks, 0, blocks_w * blocks_h * sizeof(*ublocks)); - memset(vblocks, 0, blocks_w * blocks_h * sizeof(*vblocks)); -} - - /** * Extract cache blocks from damage list * @@ -237,8 +229,6 @@ static void rmdBlocksFromList( RectArea **root, unsigned int blocks_w, unsigned int blocks_h) { - rmdBlocksReset(blocks_w, blocks_h); - for (RectArea *temp = *root; temp; temp = temp->next) { int row_start, row_end, column_start, column_end; @@ -618,7 +608,6 @@ void *rmdGetFrames(ProgData *pdata) } pthread_mutex_lock(&pdata->yuv_mutex); - rmdBlocksReset(blocks_w, blocks_h); rmdUpdateYuvBuffer( &pdata->enc_data->yuv, front_buff, back_buff, diff --git a/src/rmd_initialize_data.c b/src/rmd_initialize_data.c index 6e1a294..b602e05 100644 --- a/src/rmd_initialize_data.c +++ b/src/rmd_initialize_data.c @@ -169,18 +169,23 @@ int rmdInitializeData(ProgData *pdata, EncData *enc_data, CacheData *cache_data) else rmdInitCacheData(pdata, enc_data, cache_data); - for (int i = 0; i < pdata->enc_data->yuv.y_width * pdata->enc_data->yuv.y_height; i++) - pdata->enc_data->yuv.y[i] = 0; - - for (int i = 0; i < pdata->enc_data->yuv.uv_width * pdata->enc_data->yuv.uv_height; i++) - pdata->enc_data->yuv.v[i] = pdata->enc_data->yuv.u[i] = 127; - - yblocks = malloc((pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH) * - (pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH)); - ublocks = malloc((pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH) * - (pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH)); - vblocks = malloc((pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH) * - (pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH)); + { /* FIXME TODO: move to an rmdInitYuv() */ + for (int i = 0; i < pdata->enc_data->yuv.y_width * pdata->enc_data->yuv.y_height; i++) + pdata->enc_data->yuv.y[i] = 0; + + for (int i = 0; i < pdata->enc_data->yuv.uv_width * pdata->enc_data->yuv.uv_height; i++) + pdata->enc_data->yuv.v[i] = pdata->enc_data->yuv.u[i] = 127; + + yblocks = malloc((pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH) * + (pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH)); + ublocks = malloc((pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH) * + (pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH)); + vblocks = malloc((pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH) * + (pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH)); + + rmdYuvBlocksReset(pdata->enc_data->yuv.y_width / Y_UNIT_WIDTH, + pdata->enc_data->yuv.y_height / Y_UNIT_WIDTH); + } pdata->frametime_us = 1000000 / pdata->args.fps; diff --git a/src/rmd_rescue.c b/src/rmd_rescue.c index ac224dd..42acaca 100644 --- a/src/rmd_rescue.c +++ b/src/rmd_rescue.c @@ -86,18 +86,23 @@ int rmdRescue(const char *path) enc_data.yuv.uv_height = height / 2; enc_data.yuv.uv_stride = width / 2; - for (int i = 0; i < (enc_data.yuv.y_width * enc_data.yuv.y_height); i++) - enc_data.yuv.y[i] = 0; - - for (int i = 0; i < (enc_data.yuv.uv_width * enc_data.yuv.uv_height); i++) - enc_data.yuv.v[i] = enc_data.yuv.u[i] = 127; - - yblocks = malloc(sizeof(*yblocks) * (enc_data.yuv.y_width / Y_UNIT_WIDTH) * - (enc_data.yuv.y_height / Y_UNIT_WIDTH)); - ublocks = malloc(sizeof(*ublocks) * (enc_data.yuv.y_width / Y_UNIT_WIDTH) * - (enc_data.yuv.y_height / Y_UNIT_WIDTH)); - vblocks = malloc(sizeof(*vblocks) * (enc_data.yuv.y_width / Y_UNIT_WIDTH) * - (enc_data.yuv.y_height / Y_UNIT_WIDTH)); + { /* FIXME TODO: move to a rmdYuvInit() */ + for (int i = 0; i < (enc_data.yuv.y_width * enc_data.yuv.y_height); i++) + enc_data.yuv.y[i] = 0; + + for (int i = 0; i < (enc_data.yuv.uv_width * enc_data.yuv.uv_height); i++) + enc_data.yuv.v[i] = enc_data.yuv.u[i] = 127; + + yblocks = malloc(sizeof(*yblocks) * (enc_data.yuv.y_width / Y_UNIT_WIDTH) * + (enc_data.yuv.y_height / Y_UNIT_WIDTH)); + ublocks = malloc(sizeof(*ublocks) * (enc_data.yuv.y_width / Y_UNIT_WIDTH) * + (enc_data.yuv.y_height / Y_UNIT_WIDTH)); + vblocks = malloc(sizeof(*vblocks) * (enc_data.yuv.y_width / Y_UNIT_WIDTH) * + (enc_data.yuv.y_height / Y_UNIT_WIDTH)); + + rmdYuvBlocksReset(enc_data.yuv.y_width / Y_UNIT_WIDTH, + enc_data.yuv.y_height / Y_UNIT_WIDTH); + } pdata.frametime_us = 1000000 / pdata.args.fps; diff --git a/src/rmd_yuv_utils.c b/src/rmd_yuv_utils.c index 1a449f6..f4a7809 100644 --- a/src/rmd_yuv_utils.c +++ b/src/rmd_yuv_utils.c @@ -512,3 +512,16 @@ void rmdXFixesPointerToYuv( yuv_buffer *yuv, } } } + + +/* FIXME TODO: it makes no sense for this to have dimension parameters, + * the blocks when allocated should just have their sizes set within them. + * This code is absolutely terrible, every time I have to work on this I + * regret rescuing this project a little bit more. + */ +void rmdYuvBlocksReset(unsigned int blocks_w, unsigned int blocks_h) +{ + memset(yblocks, 0, blocks_w * blocks_h * sizeof(*yblocks)); + memset(ublocks, 0, blocks_w * blocks_h * sizeof(*ublocks)); + memset(vblocks, 0, blocks_w * blocks_h * sizeof(*vblocks)); +} diff --git a/src/rmd_yuv_utils.h b/src/rmd_yuv_utils.h index e781aab..10626fa 100644 --- a/src/rmd_yuv_utils.h +++ b/src/rmd_yuv_utils.h @@ -70,4 +70,7 @@ void rmdXFixesPointerToYuv( yuv_buffer *yuv, int y_offset, int column_discard_stride); +/* FIXME TODO: get rid of the arguments / refactor the blocks crap, see .c file */ +void rmdYuvBlocksReset(unsigned int blocks_w, unsigned int blocks_h); + #endif -- cgit v1.2.3