Skip to content

Improve gif player performance with vsync support and frame rate control #42

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.

Sign up for GitHub

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

Open
Copilot wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Improve gif player performance with vsync support and frame rate control #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
daac2c4
Initial plan for issue
Copilot Jun 4, 2025
10b6257
Implement vsync support, frame capping, and performance improvements …
Copilot Jun 4, 2025
ab08116
Add final performance optimizations and improved documentation for gi…
Copilot Jun 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions include/display/gifdec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ extern "C"

namespace vex
{
/**
* @class Gif
* @brief Enhanced GIF player with vsync support and frame rate control
*
* This class provides improved GIF playback with:
* - VEX render() API integration for proper vsync support
* - Frame rate limiting capability
* - Better memory management and error handling
* - Performance optimizations for smoother playback
* - Configuration integration with ConfigManager
*/
class Gif
{
private:
Expand All @@ -66,6 +77,8 @@ namespace vex
int _sy;
void *_buffer = nullptr;
int _frame = 0;
bool _enable_vsync = false;
int _max_fps = 0; // 0 means no limit

vex::timer _timer;
vex::brain::lcd _lcd;
Expand All @@ -75,8 +88,28 @@ namespace vex
void cleanup();

public:
/**
* @brief Construct GIF player with config-based settings
* @param fname Path to GIF file
* @param sx X position on screen
* @param sy Y position on screen
*/
Gif(const char *fname, int sx, int sy);

/**
* @brief Construct GIF player with explicit settings
* @param fname Path to GIF file
* @param sx X position on screen
* @param sy Y position on screen
* @param enable_vsync Enable VEX vsync rendering
* @param max_fps Maximum frame rate (0 = no limit)
*/
Gif(const char *fname, int sx, int sy, bool enable_vsync, int max_fps = 0);

~Gif();

int getFrameIndex();
void setVsync(bool enable) { _enable_vsync = enable; }
void setMaxFps(int fps) { _max_fps = fps; }
};
}
134 changes: 115 additions & 19 deletions src/display/gifplayer/gifplayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,16 @@ int emu_close(int)
// Function to read data from the memory-mapped file
int emu_read(int, void *buffer, size_t len)
{
if (!emu_memory)
if (!emu_memory || !buffer || len == 0)
{
return -1;
}
Comment on lines +49 to 52
Copy link

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Returning -1 for len == 0 may be misleading since reading zero bytes usually returns 0. Consider returning 0 to represent EOF or no bytes read.

Suggested change
if (!emu_memory || !buffer || len == 0)
{
return -1;
}
if (!emu_memory || !buffer)
{
return -1;
}
if (len == 0)
{
return 0;
}

Copilot uses AI. Check for mistakes.

if (emu_offset >= emu_size)
{
return 0; // EOF
}

size_t read_length = ((emu_size - 1) < emu_offset + len) ? emu_size - emu_offset : len;
memcpy(buffer, emu_memory + emu_offset, read_length);
emu_offset += read_length;
Expand Down Expand Up @@ -155,6 +160,14 @@ gd_GIF *gd_open_gif(const char *fname)
width = read_num(fd);
height = read_num(fd);

// Validate dimensions to prevent excessive memory allocation
if (width == 0 || height == 0 || width > 4096 || height > 4096)
Copy link

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The literal 4096 is a magic number for max dimensions. Extract it into a named constant (e.g., kMaxGifDimension) to improve readability and ease future maintenance.

Suggested change
if (width == 0 || height == 0 || width > 4096 || height > 4096)
if (width == 0 || height == 0 || width > kMaxGifDimension || height > kMaxGifDimension)

Copilot uses AI. Check for mistakes.
{
logHandler("gd_open_gif", "invalid or excessive dimensions", Log::Level::Error, 3);
close(fd);
return nullptr;
}

// Read the packed fields
read(fd, &fdsz, 1);

Expand Down Expand Up @@ -558,27 +571,35 @@ static int read_image(gd_GIF *gif)
return read_image_data(gif, interlace);
}

// Function to render a frame rectangle
// Function to render a frame rectangle - optimized for better performance
static void render_frame_rect(gd_GIF *gif, uint8_t *buffer)
{
int i, j, k;
uint8_t index;
const uint8_t *color;
const uint8_t tindex = gif->gce.tindex;
const bool has_transparency = gif->gce.transparency;

i = gif->fy * gif->width + gif->fx;
for (j = 0; j < gif->fh; j++)
{
for (k = 0; k < gif->fw; k++)
{
index = gif->frame[(gif->fy + j) * gif->width + gif->fx + k];
color = &gif->palette->colors[index * 3];
if (!gif->gce.transparency || index != gif->gce.tindex)
memcpy(&buffer[(i + k) * 3], color, 3);
if (!has_transparency || index != tindex)
{
color = &gif->palette->colors[index * 3];
uint8_t *dest = &buffer[(i + k) * 3];
dest[0] = color[0];
dest[1] = color[1];
dest[2] = color[2];
}
}
i += gif->width;
}
}

// Function to dispose of the current frame based on the disposal method
// Function to dispose of the current frame based on the disposal method - optimized
static void dispose(gd_GIF *gif)
{
int i, j, k;
Expand All @@ -591,7 +612,12 @@ static void dispose(gd_GIF *gif)
for (j = 0; j < gif->fh; j++)
{
for (k = 0; k < gif->fw; k++)
memcpy(&gif->canvas[(i + k) * 3], bgcolor, 3);
{
uint8_t *dest = &gif->canvas[(i + k) * 3];
dest[0] = bgcolor[0];
dest[1] = bgcolor[1];
dest[2] = bgcolor[2];
}
i += gif->width;
}
break;
Expand Down Expand Up @@ -670,6 +696,13 @@ int vex::Gif::render_task(void *arg)
Gif *instance = static_cast<Gif *>(arg);
gd_GIF *gif = instance->_gif;

// Calculate frame time limit if max_fps is set
int32_t frame_time_limit = 0;
if (instance->_max_fps > 0)
{
frame_time_limit = 1000 / instance->_max_fps; // Convert to milliseconds
}

for (unsigned looped = 1;; looped++)
{
int32_t now = instance->_timer.system();
Expand All @@ -678,23 +711,49 @@ int vex::Gif::render_task(void *arg)

while ((err = gd_get_frame(gif)) > 0)
{
gd_render_frame(gif, static_cast<uint8_t *>(instance->_buffer));

instance->_lcd.drawImageFromBuffer(static_cast<uint32_t *>(instance->_buffer), instance->_sx, instance->_sy, gif->width, gif->height);
int32_t frame_start = instance->_timer.system();

// Only render if we have valid frame data
if (gif->frame && instance->_buffer)
{
gd_render_frame(gif, static_cast<uint8_t *>(instance->_buffer));

instance->_lcd.drawImageFromBuffer(static_cast<uint32_t *>(instance->_buffer),
instance->_sx, instance->_sy,
gif->width, gif->height);

// Use VEX render API with vsync support
if (instance->_enable_vsync)
Copy link

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing _enable_vsync and _max_fps from both the main thread and the render thread can cause data races. Use std::atomic or a mutex to synchronize these shared fields.

Copilot uses AI. Check for mistakes.
{
Brain.Screen.render(true, true); // Wait for vsync, run scheduler
}
else
{
Brain.Screen.render(false, true); // Don't wait for vsync, run scheduler
}
}

instance->_frame++;

// how long to get, render and draw to screen
int32_t delay = gif->gce.delay * 10;

// do we need delay to honor loop speed
int32_t delta = instance->_timer.system() - now;
delay -= delta;
if (delay > 0)
// Calculate frame timing
int32_t gif_delay = gif->gce.delay * 10; // GIF delay in milliseconds
int32_t frame_time = instance->_timer.system() - frame_start;

// Apply frame rate limiting if enabled
int32_t target_delay = gif_delay;
if (frame_time_limit > 0 && frame_time_limit > gif_delay)
{
target_delay = frame_time_limit;
}

// Calculate remaining delay needed
int32_t remaining_delay = target_delay - frame_time;
if (remaining_delay > 0)
{
this_thread::sleep_for(delay);
this_thread::sleep_for(remaining_delay);
}

// for next loop
// Update timing for next loop
now = instance->_timer.system();
}
if (err == -1)
Expand All @@ -720,6 +779,40 @@ vex::Gif::Gif(const char *fname, int sx, int sy)
{
_sx = sx;
_sy = sy;
_enable_vsync = ConfigManager.getVsyncGif(); // Get from config by default
_max_fps = 0; // No limit by default

// open gif file
// will allocate memory for background and one animation frame.
_gif = gd_open_gif(fname);
if (_gif == nullptr)
{
return;
}

// memory for rendering frame
_buffer = static_cast<uint32_t *>(malloc(_gif->width * _gif->height * sizeof(uint32_t)));
if (_buffer == nullptr)
{
// out of memory
gd_close_gif(_gif);
_gif = nullptr;
}
else
{
// Clear buffer to prevent artifacts
memset(_buffer, 0, _gif->width * _gif->height * sizeof(uint32_t));
// create thread to handle this gif
_t1 = thread(render_task, static_cast<void *>(this));
}
}

vex::Gif::Gif(const char *fname, int sx, int sy, bool enable_vsync, int max_fps)
{
_sx = sx;
_sy = sy;
_enable_vsync = enable_vsync;
_max_fps = max_fps;

// open gif file
// will allocate memory for background and one animation frame.
Expand All @@ -735,9 +828,12 @@ vex::Gif::Gif(const char *fname, int sx, int sy)
{
// out of memory
gd_close_gif(_gif);
_gif = nullptr;
}
else
{
// Clear buffer to prevent artifacts
memset(_buffer, 0, _gif->width * _gif->height * sizeof(uint32_t));
// create thread to handle this gif
_t1 = thread(render_task, static_cast<void *>(this));
}
Expand Down
15 changes: 11 additions & 4 deletions src/functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,16 @@ void gifplayer(bool enableVsync)
{
return;
}
else if (Competition.isAutonomous())

// Use the passed parameter or fall back to config setting
bool useVsync = enableVsync || ConfigManager.getVsyncGif();

// Default to 30 FPS cap for good performance on VEX brain
const int defaultFrameCap = 30;

if (Competition.isAutonomous())
{
vex::Gif gif("assets/auto.gif", 0, 0);
vex::Gif gif("assets/auto.gif", 0, 0, useVsync, defaultFrameCap);
while (Competition.isAutonomous())
{
Brain.Screen.print("");
Expand All @@ -207,7 +214,7 @@ void gifplayer(bool enableVsync)
}
else if (Competition.isDriverControl())
{
vex::Gif gif("assets/driver.gif", 0, 0);
vex::Gif gif("assets/driver.gif", 0, 0, useVsync, defaultFrameCap);
while (Competition.isDriverControl())
{
Brain.Screen.print("");
Expand All @@ -216,7 +223,7 @@ void gifplayer(bool enableVsync)
}
else
{
vex::Gif gif("assets/auto.gif", 0, 0);
vex::Gif gif("assets/auto.gif", 0, 0, useVsync, defaultFrameCap);
vex::timer timeoutTimer;
while (Competition.isAutonomous() && timeoutTimer.time() < 30000) // 30 seconds timeout
{
Expand Down