Re: DataCache n_channels alignment
- From: Tim Janik <timj gtk org>
- To: Stefan Westerfeld <stefan space twc de>
- Cc: beast gnome org
- Subject: Re: DataCache n_channels alignment
- Date: Tue, 13 Dec 2005 23:12:06 +0100 (CET)
On Wed, 7 Dec 2005, Stefan Westerfeld wrote:
Hi!
CVS Beast doesn't support files properly that have n_channels != 2^N.
To reproduce the problem, create a simple 3-channel ogg, load it into
the wave repo, open the editor and hit play. Result:
$ beast
[...]
BSE-ERROR **: file gslwavechunk.c: line 528 (gsl_wave_chunk_use_block):
assertion failed: (block->length > 0)
hum, looks odd for the situation you describe...
This happens because the data cache nodes contain "half" frames in such
cases. Simple example (node_size = 8, n_channels = 3):
<-- 1. node --> <-- 2. node -->
| 0 1 2 0 1 2 0 1 | 2 0 1 2 0 1 2 0 |
\____/
frame that spans
more than one node
The GslWaveChunk code can't deal with that and fails. The attached patch
fixes the problem by ensuring that the data cache node size is
n_channels aligned. Please review.
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/beast/ChangeLog,v
retrieving revision 1.911
diff -u -p -r1.911 ChangeLog
--- ChangeLog 4 Dec 2005 17:15:13 -0000 1.911
+++ ChangeLog 7 Dec 2005 14:53:23 -0000
@@ -1,3 +1,8 @@
+Wed Dec 7 15:24:12 2005 Stefan Westerfeld <stefan space twc de>
+
+ * tools/bseloopfuncs.c: Adapt to the data cache API change: pass
+ additional n_channels argument.
+
Sun Dec 4 18:07:03 2005 Stefan Westerfeld <stefan space twc de>
* configure.in:
Index: bse/ChangeLog
===================================================================
RCS file: /cvs/gnome/beast/bse/ChangeLog,v
retrieving revision 1.588
diff -u -p -r1.588 ChangeLog
--- bse/ChangeLog 1 Dec 2005 12:24:44 -0000 1.588
+++ bse/ChangeLog 7 Dec 2005 14:53:28 -0000
@@ -1,3 +1,17 @@
+Wed Dec 7 15:18:42 2005 Stefan Westerfeld <stefan space twc de>
+
+ * gsldatacache.[hc]: Extend the data cache API to allow specifying
+ n_channels. Ensure that the data cache node size is n_channels
+ aligned, so that data cache nodes always contain whole frames, and no
+ frames wrap around the end of one data cache node and continue within
+ the next.
+
+ * tests/testwavechunk.c:
+ * bseloader.c:
+ * bsewave.c:
+ * gsldatautils.c: Adapt to the data cache API change: pass additional
+ n_channels argument.
+
Thu Nov 17 18:10:56 2005 Stefan Westerfeld <stefan space twc de>
* gslfilter.[hc]: Added new argument to gsl_filter_fir_approx, to
Index: bse/bseloader.c
===================================================================
RCS file: /cvs/gnome/beast/bse/bseloader.c,v
retrieving revision 1.23
diff -u -p -r1.23 bseloader.c
--- bse/bseloader.c 19 Apr 2005 14:13:12 -0000 1.23
+++ bse/bseloader.c 7 Dec 2005 14:53:29 -0000
@@ -373,7 +373,7 @@ bse_wave_chunk_create (BseWaveDsc *wav
/* FIXME: we essentially create a dcache for each wchunk here ;( */
- dcache = gsl_data_cache_from_dhandle (dhandle, gsl_get_config ()->wave_chunk_padding * wave_dsc->n_channels);
+ dcache = gsl_data_cache_from_dhandle (dhandle, gsl_get_config ()->wave_chunk_padding * wave_dsc->n_channels, wave_dsc->n_channels);
hm, this looks like you now need to make sure that the padding is
n_channels algined. with the new n_channels argument passed in,
i'd rather see us specify per-channel padding, and also reorder
arguments, so we we get:
GslDataCache* gsl_data_cache_new (GslDataHandle *dhandle,
guint n_channels,
guint per_channel_padding);
gsl_data_handle_unref (dhandle);
if (!dcache)
return NULL;
Index: bse/bsewave.c
===================================================================
RCS file: /cvs/gnome/beast/bse/bsewave.c,v
retrieving revision 1.38
diff -u -p -r1.38 bsewave.c
--- bse/bsewave.c 28 Jul 2005 12:05:50 -0000 1.38
+++ bse/bsewave.c 7 Dec 2005 14:53:29 -0000
@@ -632,7 +632,8 @@ bse_wave_restore_private (BseObject *ob
gsl_data_handle_unref (tmp_handle);
}
GslDataCache *dcache = gsl_data_cache_from_dhandle (parsed_wchunk.data_handle,
- gsl_get_config ()->wave_chunk_padding * parsed_wchunk.wh_n_channels);
+ gsl_get_config ()->wave_chunk_padding * parsed_wchunk.wh_n_channels,
+ parsed_wchunk.wh_n_channels);
dito
const gchar *ltype = bse_xinfos_get_value (parsed_wchunk.xinfos, "loop-type");
GslWaveLoopType loop_type = ltype ? gsl_wave_loop_type_from_string (ltype) : GSL_WAVE_LOOP_NONE;
SfiNum loop_start = bse_xinfos_get_num (parsed_wchunk.xinfos, "loop-start");
Index: bse/gsldatacache.c
===================================================================
RCS file: /cvs/gnome/beast/bse/gsldatacache.c,v
retrieving revision 1.18
diff -u -p -r1.18 gsldatacache.c
--- bse/gsldatacache.c 31 Dec 2004 12:46:26 -0000 1.18
+++ bse/gsldatacache.c 7 Dec 2005 14:53:29 -0000
@@ -88,16 +88,32 @@ _gsl_init_data_caches (void)
GslDataCache*
gsl_data_cache_new (GslDataHandle *dhandle,
- guint padding)
+ guint padding,
+ guint n_channels)
dito
{
- guint node_size = CONFIG_NODE_SIZE () / sizeof (GslDataType);
+ /*
+ * We ensure that (node_size % n_channels) == 0, so that data cache nodes
+ * always contain whole frames, and no frames wrap around the end of one
+ * data cache node and continue within the next.
+ *
+ * The default node_size is 840, because 840 = 2 * 2 * 2 * 3 * 5 * 7.
+ *
+ * This is suitable for many common values of n_channels, such as 1-8, 10,
+ * 12, 24, and of course many others. For other values of n_channels (such
+ * as 9), we adapt the node_size below.
+ */
+ guint node_size = 840;
GslDataCache *dcache;
hm, 840 * 4 (sizeof float) = 3360. roughly 5/6th of a page,
sounds halfway reasonable.
g_return_val_if_fail (dhandle != NULL, NULL);
g_return_val_if_fail (padding > 0, NULL);
g_return_val_if_fail (dhandle->name != NULL, NULL);
- g_assert (node_size == sfi_alloc_upper_power2 (node_size));
- g_return_val_if_fail (padding < node_size / 2, NULL);
we give up node_size being a power of 2 here. however the old
code was buggy in implementing that anway, and there was never
a strong reason for enforcing this.
+ g_return_val_if_fail (padding % n_channels == 0, NULL);
ah, the assertion i saw coming above... ;)
+ g_return_val_if_fail (n_channels <= node_size, NULL);
hmmm...
840 is probably a resonable upper channel limit.
however we need corresponding checks at relevant GUI interfaces
to properly deal with waves that have too many channels (so we show
an error/warning there instead of crashing).
that means we need to determine a fixed limit here, and export it
somewhere (maybe per GslConfig) and at least identify all GUI interfaces
affected by this. (identifying the relevant GUI/UI portions is
good enough for you, the implementation, et least for the GUI parts
can be left up to me if you don't want to tackle it, since i'm probably
more experienced with the GUI stuff).
+
+ /* adapt node_size to be n_channels aligned */
+ node_size /= n_channels;
+ node_size *= n_channels;
/* allocate new closed dcache if necessary */
dcache = sfi_new_struct (GslDataCache, 1);
@@ -307,7 +323,7 @@ data_cache_new_node_L (GslDataCache *dca
g_memmove (node_p + 1, node_p, (i - pos) * sizeof (*node_p));
dnode = sfi_new_struct (GslDataCacheNode, 1);
(*node_p) = dnode;
- dnode->offset = offset & ~(dcache->node_size - 1);
+ dnode->offset = offset - (offset % dcache->node_size);
dnode->ref_count = 1;
dnode->age = 0;
dnode->data = NULL;
@@ -545,13 +561,13 @@ gsl_data_cache_unref_node (GslDataCache
if (check_cache)
{
- guint node_size = CONFIG_NODE_SIZE ();
+ guint node_mem_size = (dcache->node_size + (dcache->padding << 1)) * sizeof (GslDataType);
i usually prefer writing *2 instead of <<1, since modern compilers can
optimize that if needed, and *2 looks more natural to most people.
guint cache_mem = gsl_get_config ()->dcache_cache_memory;
guint current_mem;
GSL_SPIN_LOCK (&global_dcache_mutex);
global_dcache_n_aged_nodes++;
- current_mem = node_size * global_dcache_n_aged_nodes;
+ current_mem = node_mem_size * global_dcache_n_aged_nodes;
if (current_mem > cache_mem) /* round-robin cache trashing */
{
guint dcache_count, needs_unlock;
@@ -575,7 +591,7 @@ gsl_data_cache_unref_node (GslDataCache
*/
current_mem -= cache_mem; /* overhang */
current_mem += cache_mem >> 4; /* overflow = overhang + 6% */
this seems to be an exception to my above rule ;)
- current_mem /= node_size; /* n_nodes to free */
+ current_mem /= node_mem_size; /* n_nodes to free */
current_mem = MIN (current_mem, dcache->n_nodes);
guint max_lru = dcache->n_nodes;
max_lru >>= 1; /* keep at least 75% of n_nodes */
@@ -594,8 +610,8 @@ gsl_data_cache_unref_node (GslDataCache
g_printerr ("shrunk dcache by: dhandle=%p - %s - highp=%d: %d bytes (kept: %d)\n",
dcache->dhandle, gsl_data_handle_name (dcache->dhandle),
dcache->high_persistency,
- -(gint) node_size * (debug_gnaged - global_dcache_n_aged_nodes),
- node_size * dcache->n_nodes);
+ -(gint) node_mem_size * (debug_gnaged - global_dcache_n_aged_nodes),
+ node_mem_size * dcache->n_nodes);
#endif
if (needs_unlock)
GSL_SPIN_UNLOCK (&dcache->mutex);
@@ -619,18 +635,20 @@ gsl_data_cache_free_olders (GslDataCache
GslDataCache*
gsl_data_cache_from_dhandle (GslDataHandle *dhandle,
- guint min_padding)
+ guint min_padding,
+ guint n_channels)
new argument order again (dhandle, n_channels, padding_per_channel).
{
SfiRing *ring;
g_return_val_if_fail (dhandle != NULL, NULL);
+ g_return_val_if_fail (n_channels > 0, NULL);
good catch.
GSL_SPIN_LOCK (&global_dcache_mutex);
for (ring = global_dcache_list; ring; ring = sfi_ring_walk (ring, global_dcache_list))
{
GslDataCache *dcache = ring->data;
- if (dcache->dhandle == dhandle && dcache->padding >= min_padding)
+ if (dcache->dhandle == dhandle && dcache->padding >= min_padding && dcache->n_channels == n_channels)
{
gsl_data_cache_ref (dcache);
GSL_SPIN_UNLOCK (&global_dcache_mutex);
@@ -639,5 +657,5 @@ gsl_data_cache_from_dhandle (GslDataHand
}
GSL_SPIN_UNLOCK (&global_dcache_mutex);
- return gsl_data_cache_new (dhandle, min_padding);
+ return gsl_data_cache_new (dhandle, min_padding, n_channels);
dito.
}
Index: bse/gsldatacache.h
===================================================================
RCS file: /cvs/gnome/beast/bse/gsldatacache.h,v
retrieving revision 1.7
diff -u -p -r1.7 gsldatacache.h
--- bse/gsldatacache.h 29 Dec 2004 03:24:43 -0000 1.7
+++ bse/gsldatacache.h 7 Dec 2005 14:53:29 -0000
@@ -36,8 +36,9 @@ struct _GslDataCache
guint open_count;
SfiMutex mutex;
guint ref_count;
- guint node_size; /* power of 2, const for all dcaches */
+ guint node_size; /* multiple of n_channels */
guint padding; /* n_values around blocks */
+ guint n_channels;
guint max_age;
gboolean high_persistency; /* valid for opened caches only */
guint n_nodes;
@@ -60,7 +61,8 @@ typedef enum
/* --- prototypes --- */
GslDataCache* gsl_data_cache_new (GslDataHandle *dhandle,
- guint padding);
+ guint padding,
+ guint n_channels);
dito.
GslDataCache* gsl_data_cache_ref (GslDataCache *dcache);
void gsl_data_cache_unref (GslDataCache *dcache);
void gsl_data_cache_open (GslDataCache *dcache);
@@ -73,7 +75,8 @@ void gsl_data_cache_unref_node (GslDa
void gsl_data_cache_free_olders (GslDataCache *dcache,
guint max_age);
GslDataCache* gsl_data_cache_from_dhandle (GslDataHandle *dhandle,
- guint min_padding);
+ guint min_padding,
+ guint n_channels);
dito.
G_END_DECLS
Index: bse/gsldatautils.c
===================================================================
RCS file: /cvs/gnome/beast/bse/gsldatautils.c,v
retrieving revision 1.23
diff -u -p -r1.23 gsldatautils.c
--- bse/gsldatautils.c 19 Apr 2005 14:13:12 -0000 1.23
+++ bse/gsldatautils.c 7 Dec 2005 14:53:30 -0000
@@ -511,7 +511,7 @@ gsl_data_find_tailmatch (GslDataHandle
return FALSE;
}
- dcache = gsl_data_cache_new (dhandle, 1);
+ dcache = gsl_data_cache_new (dhandle, 1, dhandle->setup.n_channels);
dito.
shandle = gsl_data_handle_new_dcached (dcache);
gsl_data_cache_unref (dcache);
gsl_data_handle_open (shandle);
Index: bse/tests/testwavechunk.c
===================================================================
RCS file: /cvs/gnome/beast/bse/tests/testwavechunk.c,v
retrieving revision 1.14
diff -u -p -r1.14 testwavechunk.c
--- bse/tests/testwavechunk.c 27 Dec 2004 23:27:59 -0000 1.14
+++ bse/tests/testwavechunk.c 7 Dec 2005 14:53:30 -0000
@@ -66,7 +66,7 @@ run_tests (GslWaveLoopType loop_type,
BseErrorType error;
myhandle = gsl_data_handle_new_mem (1, 32, 44100, 440, my_data_length, my_data, NULL);
- dcache = gsl_data_cache_new (myhandle, 1);
+ dcache = gsl_data_cache_new (myhandle, 1, 1);
dito... nah ;)
gsl_data_handle_unref (myhandle);
wchunk = gsl_wave_chunk_new (dcache,
44100.0, 44.0,
Index: tools/bseloopfuncs.c
===================================================================
RCS file: /cvs/gnome/beast/tools/bseloopfuncs.c,v
retrieving revision 1.8
diff -u -p -r1.8 bseloopfuncs.c
--- tools/bseloopfuncs.c 1 Dec 2005 12:21:08 -0000 1.8
+++ tools/bseloopfuncs.c 7 Dec 2005 14:53:31 -0000
@@ -798,7 +798,7 @@ gsl_data_find_loop1 (GslDataHandle *d
if (config->repetitions != CLAMP (config->repetitions, 2, config->block_length))
return FALSE;
- dcache = gsl_data_cache_new (dhandle, 1);
+ dcache = gsl_data_cache_new (dhandle, 1, 1);
gsl_data_cache_open (dcache);
gsl_data_handle_close (dhandle);
gsl_data_cache_unref (dcache);
@@ -1018,7 +1018,7 @@ gsl_data_find_loop0 (GslDataHandle
g_return_val_if_fail (cfg->cmp_strategy == GSL_DATA_TAIL_LOOP_CMP_LEAST_SQUARE ||
cfg->cmp_strategy == GSL_DATA_TAIL_LOOP_CMP_CORRELATION, 0);
- dcache = gsl_data_cache_new (dhandle, 1);
+ dcache = gsl_data_cache_new (dhandle, 1, 1);
gsl_data_cache_open (dcache);
gsl_data_handle_close (dhandle);
gsl_data_cache_unref (dcache);
thanks, your fixes basically look very good. we just need to
work out the details now, and the nasty UI bits ;)
Cu... Stefan
--
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan
---
ciaoTJ
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
