[gimp-gap] player cache limitation now supports higher values than 2047 MB
- From: Wolfgang Hofer <wolfgangh src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gimp-gap] player cache limitation now supports higher values than 2047 MB
- Date: Sat, 7 Apr 2018 09:40:59 +0000 (UTC)
commit 133bd576da49ee5e0d8005d6a52e386dd604b252
Author: Wolfgang Hofer <wolfgangh svn gnome org>
Date: Sat Apr 7 11:40:33 2018 +0200
player cache limitation now supports higher values than 2047 MB
ChangeLog | 11 +++++
gap/gap_player_cache.c | 74 +++++++++++++++++++++++-----------
gap/gap_player_cache.h | 11 +++--
gap/gap_player_dialog.c | 100 ++++++++++++++++++++++++++++++++++++++++-------
gap/gap_player_main.h | 2 +-
gap/gap_pview_da.c | 26 +++++++++---
gap/gap_pview_da.h | 3 +-
7 files changed, 175 insertions(+), 52 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index bb0e3e8..233b58a 100755
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2018-04-07 Wolfgang Hofer <hof gimp org>
+
+- player cache limitation now supports higher values than 2047 MB
+ Note: the player still will crash in case
+ the user sets the limit greater than the available memory.
+
+ * gap/gap_pview_da.c [.h]
+ * gap/gap_player_main.h
+ * gap/gap_player_dialog.c
+ * gap/gap_player_cache.c [h]
+
2018-01-25 Wolfgang Hofer <hof gimp org>
- use larger spinbutton entries in the MovePath Dialog and bigger inital preview size.
diff --git a/gap/gap_player_cache.c b/gap/gap_player_cache.c
index 8e8fc3f..26b1d29 100644
--- a/gap/gap_player_cache.c
+++ b/gap/gap_player_cache.c
@@ -115,8 +115,8 @@ typedef struct GapPlayerCacheElem {
typedef struct GapPlayerCacheAdmin { /* nick: admin_ptr */
GapPlayerCacheElem *start_elem; /* recently accessed element */
GapPlayerCacheElem *end_elem; /* last list elem (access long ago) */
- gint32 summary_bytesize;
- gint32 configured_max_bytesize;
+ gint64 summary_bytesize;
+ gint64 configured_max_bytesize;
GHashTable *pcache_elemref_hash;
} GapPlayerCacheAdmin;
@@ -129,7 +129,7 @@ static GapPlayerCacheAdmin* p_get_admin_ptr(void);
static void p_debug_printf_cache_list(GapPlayerCacheAdmin *admin_ptr);
static void p_debug_printf_cdata(GapPlayerCacheData *cdata);
-static void p_player_cache_shrink(GapPlayerCacheAdmin *admin_ptr, gint32 new_bytesize);
+static void p_player_cache_shrink(GapPlayerCacheAdmin *admin_ptr, gint64 new_bytesize);
static void p_player_cache_remove_oldest_frame(GapPlayerCacheAdmin *admin_ptr);
static void p_free_elem(GapPlayerCacheElem *delete_elem_ptr);
static void p_player_cache_add_new_frame(GapPlayerCacheAdmin *admin_ptr
@@ -160,12 +160,19 @@ p_get_admin_ptr(void)
admin_ptr->end_elem = NULL;
admin_ptr->summary_bytesize = 0;
admin_ptr->configured_max_bytesize = GAP_PLAYER_CACHE_DEFAULT_MAX_BYTESIZE;
-
+ admin_ptr->pcache_elemref_hash = NULL;
+ }
+
+ if (global_pca_ptr->pcache_elemref_hash == NULL)
+ {
+ global_pca_ptr->start_elem = NULL;
+ global_pca_ptr->end_elem = NULL;
+ global_pca_ptr->summary_bytesize = 0;
/* use NULL to skip destructor for the ckey
* because the ckey is also part of the value
* (and therefore freed via the value destuctor procedure p_free_elem)
*/
- admin_ptr->pcache_elemref_hash = g_hash_table_new_full(g_str_hash
+ global_pca_ptr->pcache_elemref_hash = g_hash_table_new_full(g_str_hash
, g_str_equal
, NULL
, (GDestroyNotify) p_free_elem
@@ -254,7 +261,7 @@ p_debug_printf_cache_list(GapPlayerCacheAdmin *admin_ptr)
* ---------------------------------
*/
void
-gap_player_cache_set_max_bytesize(gint32 max_bytesize)
+gap_player_cache_set_max_bytesize(gint64 max_bytesize)
{
GapPlayerCacheAdmin *admin_ptr;
@@ -280,11 +287,11 @@ gap_player_cache_set_max_bytesize(gint32 max_bytesize)
* gap_player_cache_get_max_bytesize
* -----------------------------------------
*/
-gint32
+gint64
gap_player_cache_get_max_bytesize(void)
{
GapPlayerCacheAdmin *admin_ptr;
- gint32 bytesize;
+ gint64 bytesize;
bytesize = 0;
admin_ptr = p_get_admin_ptr();
@@ -300,11 +307,11 @@ gap_player_cache_get_max_bytesize(void)
* gap_player_cache_get_current_bytes_used
* -----------------------------------------
*/
-gint32
+gint64
gap_player_cache_get_current_bytes_used(void)
{
GapPlayerCacheAdmin *admin_ptr;
- gint32 bytesize;
+ gint64 bytesize;
bytesize = 0;
admin_ptr = p_get_admin_ptr();
@@ -340,10 +347,10 @@ gap_player_cache_get_current_frames_cached(void)
* gap_player_cache_get_gimprc_bytesize
* ------------------------------------
*/
-gint32
+gint64
gap_player_cache_get_gimprc_bytesize(void)
{
- gint32 bytesize;
+ gint64 bytesize;
gchar *value_string;
value_string = gimp_gimprc_query("video_playback_cache");
@@ -392,10 +399,10 @@ gap_player_cache_get_gimprc_bytesize(void)
* ------------------------------------
*/
void
-gap_player_cache_set_gimprc_bytesize(gint32 bytesize)
+gap_player_cache_set_gimprc_bytesize(gint64 bytesize)
{
- gint32 kbsize;
- gint32 mbsize;
+ gint64 kbsize;
+ gint64 mbsize;
gchar *value_string;
kbsize = bytesize / 1024;
@@ -424,10 +431,10 @@ gap_player_cache_set_gimprc_bytesize(gint32 bytesize)
* p_get_elem_size
* ------------------------------
*/
-static gint32
+static gint64
p_get_elem_size(GapPlayerCacheElem *elem_ptr)
{
- gint32 bytesize;
+ gint64 bytesize;
bytesize = 0;
@@ -536,9 +543,14 @@ gap_player_cache_free_all(void)
p_player_cache_remove_oldest_frame(admin_ptr);
}
g_hash_table_destroy(admin_ptr->pcache_elemref_hash);
-
- global_pca_ptr = NULL;
- g_free(admin_ptr);
+ admin_ptr->pcache_elemref_hash = NULL;
+ admin_ptr->summary_bytesize = 0;
+
+ /* keep the global_pca_ptr for the full session length
+ * (otherwise it would lose configured configured_max_bytesize setting)
+ */
+ // global_pca_ptr = NULL;
+ // g_free(admin_ptr);
}
} /* end gap_player_cache_free_all */
@@ -551,7 +563,7 @@ gap_player_cache_free_all(void)
* until fit to configured maximum
*/
static void
-p_player_cache_shrink(GapPlayerCacheAdmin *admin_ptr, gint32 new_bytesize)
+p_player_cache_shrink(GapPlayerCacheAdmin *admin_ptr, gint64 new_bytesize)
{
while(admin_ptr->start_elem != NULL)
{
@@ -569,6 +581,11 @@ p_player_cache_shrink(GapPlayerCacheAdmin *admin_ptr, gint32 new_bytesize)
}
} /* end p_player_cache_shrink */
+void
+gap_player_cache_remove_oldest_frame(void)
+{
+ p_player_cache_remove_oldest_frame(p_get_admin_ptr());
+}
/* ----------------------------------
* p_player_cache_remove_oldest_frame
@@ -778,7 +795,7 @@ gap_player_cache_insert(const gchar *ckey
, GapPlayerCacheData *cdata)
{
GapPlayerCacheAdmin *admin_ptr;
- gint32 new_bytesize;
+ gint64 new_bytesize;
GapPlayerCacheElem *new_elem_ptr;
if((ckey == NULL) || (cdata == NULL))
@@ -798,6 +815,11 @@ gap_player_cache_insert(const gchar *ckey
{
printf("gap_player_cache_insert: INSERT REJECTED! ckey:<%s> \n", ckey);
}
+ /* free the rejected cdata just in case ...
+ * .. but REJECT should not occure (and did not occure in my tests)
+ * .. because the player shall not attempt to add elements more than once.
+ */
+ gap_player_cache_free_cdata(cdata);
return;
}
@@ -854,7 +876,7 @@ gap_player_cache_decompress(GapPlayerCacheData *cdata)
* create and set up a new player chache data stucture.
* NOTE: The th_data is NOT copied but used as 1:1 reference
* in case no compression is done.
- * comression (not implemented yet) will create
+ * compression (not implemented yet) will create
* a compressed copy of th_data (that is put into the newly created
* GapPlayerCacheData structure, and g_free th_data after the compression.
*/
@@ -870,7 +892,11 @@ gap_player_cache_new_data(guchar *th_data
{
GapPlayerCacheData* cdata;
- cdata = g_new ( GapPlayerCacheData, 1 );
+ cdata = g_try_malloc ( sizeof(GapPlayerCacheData) );
+ if (cdata == NULL)
+ {
+ return (NULL);
+ }
cdata->compression = compression;
cdata->th_data_size = th_size;
cdata->th_width = th_width;
diff --git a/gap/gap_player_cache.h b/gap/gap_player_cache.h
index ec34e49..504880e 100644
--- a/gap/gap_player_cache.h
+++ b/gap/gap_player_cache.h
@@ -64,16 +64,17 @@ typedef struct GapPlayerCacheData {
-gint32 gap_player_cache_get_max_bytesize(void);
-gint32 gap_player_cache_get_current_bytes_used(void);
+gint64 gap_player_cache_get_max_bytesize(void);
+gint64 gap_player_cache_get_current_bytes_used(void);
gint32 gap_player_cache_get_current_frames_cached(void);
-gint32 gap_player_cache_get_gimprc_bytesize(void);
+gint64 gap_player_cache_get_gimprc_bytesize(void);
-void gap_player_cache_set_gimprc_bytesize(gint32 bytesize);
-void gap_player_cache_set_max_bytesize(gint32 max_bytesize);
+void gap_player_cache_set_gimprc_bytesize(gint64 bytesize);
+void gap_player_cache_set_max_bytesize(gint64 max_bytesize);
GapPlayerCacheData* gap_player_cache_lookup(const gchar *ckey);
void gap_player_cache_insert(const gchar *ckey
, GapPlayerCacheData *data);
+void gap_player_cache_remove_oldest_frame(void);
guchar* gap_player_cache_decompress(GapPlayerCacheData *cdata);
GapPlayerCacheData* gap_player_cache_new_data(guchar *th_data
diff --git a/gap/gap_player_dialog.c b/gap/gap_player_dialog.c
index 47fe157..7c47933 100644
--- a/gap/gap_player_dialog.c
+++ b/gap/gap_player_dialog.c
@@ -3399,10 +3399,13 @@ p_frame_chache_processing(GapPlayerMainGlobalParams *gpp
{
GapPlayerCacheData *cdata;
guchar *th_data;
+ guchar *th_pixel_data_copy;
+ size_t pixel_data_size;
gint32 th_size;
gint32 th_width;
gint32 th_height;
gint32 th_bpp;
+ gint32 countFramesInCache;
gboolean th_has_alpha;
@@ -3416,17 +3419,45 @@ p_frame_chache_processing(GapPlayerMainGlobalParams *gpp
/* frame with ckey is already cached */
return;
}
+
+ pixel_data_size = gpp->pv_ptr->pv_height * gpp->pv_ptr->pv_width * gpp->pv_ptr->pv_bpp;
+
+ th_pixel_data_copy = NULL;
+ cdata = NULL;
+
+ countFramesInCache = gap_player_cache_get_current_frames_cached();
- th_data = gap_pview_get_repaint_thdata(gpp->pv_ptr
+ /* loop with attempts to add a copy of the frame (that is already rendered in the pview widget)
+ * to the players cache.
+ * (this loop tries to eliminate older elements when running out of memory)
+ */
+ while(TRUE)
+ {
+ if (th_pixel_data_copy == NULL)
+ {
+ th_pixel_data_copy = g_try_malloc (pixel_data_size);
+ }
+ if (th_pixel_data_copy)
+ {
+ th_data = gap_pview_get_copy_of_repaint_thdata(gpp->pv_ptr
, &th_size
, &th_width
, &th_height
, &th_bpp
, &th_has_alpha
+ , th_pixel_data_copy /* preallocated buffer is filled when repaint data is
available */
);
- if (th_data != NULL)
- {
- cdata = gap_player_cache_new_data(th_data
+ if(th_data == NULL)
+ {
+ /* throw away the preallocated buffer that could not be filled with useful data
+ * and return without adding to the cache
+ */
+ g_free(th_pixel_data_copy);
+ return;
+ }
+
+
+ cdata = gap_player_cache_new_data(th_pixel_data_copy
, th_size
, th_width
, th_height
@@ -3434,13 +3465,54 @@ p_frame_chache_processing(GapPlayerMainGlobalParams *gpp
, gpp->cache_compression
, gpp->pv_ptr->flip_status
);
- if(cdata != NULL)
+ if(cdata != NULL)
+ {
+ /* insert frame into cache
+ */
+ gap_player_cache_insert(ckey, cdata);
+ p_update_cache_status(gpp);
+
+ /* now the new elemnts are part of the cache
+ * (and will be freed later under control of the hash table)
+ */
+ return;
+ }
+ }
+
+ if (countFramesInCache <= 0)
{
- /* insert frame into cache
- */
- gap_player_cache_insert(ckey, cdata);
- p_update_cache_status(gpp);
+ break;
}
+
+ if(gap_debug)
+ {
+ printf("p_frame_chache_processing: next attempt to add cache element with countFramesInCache:%d",
countFramesInCache);
+ }
+
+ /* attempts to add the new element faild (due to lack of memory),
+ * but there are older cached elements that can be removed
+ * to get back some memory for another attempt.
+ * Note:
+ * in tests this situation was not reached, because the player cache
+ * is not the only memory allocating application part, and the "out of memory"
+ * may occure (after ram and swap are filled up) on any other places...
+ * .. in other words: this current implementation still crashes when
+ * the configured cache limit was set greater than the available memory.
+ */
+ gap_player_cache_remove_oldest_frame();
+ countFramesInCache--;
+ }
+
+ /* at this point we did not get memory for adding the element to the cache
+ * (even after all cached elements were removed)
+ */
+ if (th_pixel_data_copy != NULL)
+ {
+ g_free(th_pixel_data_copy);
+ }
+ if (cdata != NULL)
+ {
+ g_free(cdata);
}
} /* end p_frame_chache_processing */
@@ -7238,8 +7310,8 @@ p_update_cache_status (GapPlayerMainGlobalParams *gpp)
{
static char status_txt[50];
gint32 elem_counter;
- gint32 bytes_used;
- gint32 max_bytes;
+ gint64 bytes_used;
+ gint64 max_bytes;
elem_counter = gap_player_cache_get_current_frames_cached();
@@ -7297,9 +7369,9 @@ on_cache_size_spinbutton_changed (GtkEditable *editable,
mb_chachesize = GTK_ADJUSTMENT(gpp->cache_size_spinbutton_adj)->value;
bytesize = mb_chachesize * (1024.0 * 1024.0);
- if(gpp->max_player_cache != (gint32)bytesize)
+ if(gpp->max_player_cache != (gint64)bytesize)
{
- gpp->max_player_cache = (gint32)bytesize;
+ gpp->max_player_cache = (gint64)bytesize;
if(gap_debug)
{
@@ -7470,7 +7542,7 @@ p_new_configframe(GapPlayerMainGlobalParams *gpp)
spinbutton = gimp_spin_button_new (&adj, /* return value */
mb_cachesize, /* initial_val */
0.0, /* umin */
- 9000.0, /* umax */
+ 32000.0, /* umax */
1.0, /* sstep */
10.0, /* pagestep */
0.0, /* page_size */
diff --git a/gap/gap_player_main.h b/gap/gap_player_main.h
index f52b51f..56bfb7e 100644
--- a/gap/gap_player_main.h
+++ b/gap/gap_player_main.h
@@ -219,7 +219,7 @@ typedef struct GapPlayerMainGlobalParams {
GtkWidget *label_current_cache_values;
GtkWidget *progress_bar_cache_usage;
- gint32 max_player_cache; /* max bytesize to use for caching frames
+ gint64 max_player_cache; /* max bytesize to use for caching frames
* (at pview widget size)
* a value of 0 turns caching OFF
*/
diff --git a/gap/gap_pview_da.c b/gap/gap_pview_da.c
index 8aa01fa..ccc0211 100644
--- a/gap/gap_pview_da.c
+++ b/gap/gap_pview_da.c
@@ -1412,19 +1412,23 @@ gap_pview_get_repaint_pixbuf(GapPView *pv_ptr)
} /* end gap_pview_get_repaint_pixbuf */
-/* ------------------------------
- * gap_pview_get_repaint_thdata
- * ------------------------------
+/* ------------------------------------
+ * gap_pview_get_copy_of_repaint_thdata
+ * ------------------------------------
* return the repaint buffer as thumbnail data (guchar RGB),
* or NULL if no buffer is available.
+ * Note that the optionally preallocated buffer th_pixel_data_copy
+ * must be large enough to hold (th_width * th_height * th_bpp) bytes,
+ * and can be freed by the caller, in case it could not be filled with useful date (indicated by returnvalue
NULL)
*/
guchar *
-gap_pview_get_repaint_thdata(GapPView *pv_ptr /* IN */
+gap_pview_get_copy_of_repaint_thdata(GapPView *pv_ptr /* IN */
, gint32 *th_size /* OUT */
, gint32 *th_width /* OUT */
, gint32 *th_height /* OUT */
, gint32 *th_bpp /* OUT */
, gboolean *th_has_alpha /* OUT */
+ , guchar *th_pixel_data_copy /* IN/OUT optional preallocated buffer must
have size (th_width * th_height * th_bpp) */
)
{
/* int bits_per_sample; */
@@ -1432,7 +1436,6 @@ gap_pview_get_repaint_thdata(GapPView *pv_ptr /* IN */
int height;
gboolean has_alpha;
guchar *pixel_data_src;
- guchar *pixel_data_copy;
if (pv_ptr == NULL)
@@ -1476,8 +1479,17 @@ gap_pview_get_repaint_thdata(GapPView *pv_ptr /* IN */
if(pixel_data_src)
{
+ guchar *pixel_data_copy;
size_t pixel_data_size = pv_ptr->pv_height * pv_ptr->pv_width * pv_ptr->pv_bpp;
- pixel_data_copy = g_new ( guchar, pixel_data_size );
+
+ if(th_pixel_data_copy)
+ {
+ pixel_data_copy = th_pixel_data_copy;
+ }
+ else
+ {
+ pixel_data_copy = g_new ( guchar, pixel_data_size );
+ }
memcpy(pixel_data_copy, pixel_data_src, pixel_data_size);
*th_size = width * height * pv_ptr->pv_bpp;
@@ -1490,4 +1502,4 @@ gap_pview_get_repaint_thdata(GapPView *pv_ptr /* IN */
}
return (NULL);
-} /* end gap_pview_get_repaint_thdata */
+} /* end gap_pview_get_copy_of_repaint_thdata */
diff --git a/gap/gap_pview_da.h b/gap/gap_pview_da.h
index d5db3d2..13d9afa 100644
--- a/gap/gap_pview_da.h
+++ b/gap/gap_pview_da.h
@@ -111,11 +111,12 @@ void gap_pview_drop_repaint_buffers(GapPView *pv_ptr);
void gap_pview_render_default_icon(GapPView *pv_ptr);
GdkPixbuf *gap_pview_get_repaint_pixbuf(GapPView *pv_ptr);
-guchar * gap_pview_get_repaint_thdata(GapPView *pv_ptr /* IN */
+guchar * gap_pview_get_copy_of_repaint_thdata(GapPView *pv_ptr /* IN */
, gint32 *th_size /* OUT */
, gint32 *th_width /* OUT */
, gint32 *th_height /* OUT */
, gint32 *th_bpp /* OUT */
, gboolean *th_has_alpha /* OUT */
+ , guchar *th_pixel_data_copy /* IN/OUT optional preallocated buffer must
have size (th_width * th_height * th_bpp) */
);
#endif
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
