hey all,
so, in IRC we somehow got onto the problem that the filebrowser needs
to know when a track changes (so the follow playlist can work
properly), and scrobbler and database also need to know when this
happens.
playback.c has a few callbacks but only allows one function to be
registered for each, which causes a bit of a mess in playback.c (and
is one reason why scrobbler is broken on hwcodec.)
My attached patch addresses this issue, by taking the existing
ata_notify_callback code and extending it so functions are registered
as callback for certain events.
so, what would happen is database, scrobbler and tree would all
register individual callbacks on the PLAYBACK_TRACK_CHANGED event,
then playback.c (or mpeg.c) would call call_event_callbacks() with the
PLAYBACK_TRACK_CHANGED event id and some known data which would then
call each registered function (for that id).
Firstly, my reasoning for having a seperate module for this is so
playback.c doesnt have the extra clutter which just makes it more
difficult to work in then it already is, It makes adding more
callbacks for an event very simple, and hopefully makes the code a bit
smaller if this is current used in a similar way in other parts of the
code?
The reason for the event id and a single array for the callbacks is
that it wastes less ram than having a seperate array for each event
group.
At the moment, all this patch does it replace the existing ata
callback stuff with this, it doesnt touch playback yet. I just wanted
to get this out and hopefully get some feedback on weather this is
good or not?
Also, which other events would be usefull to add?
Questions, comments?
cheers,
Jonathan
Index: apps/settings.c
===================================================================
--- apps/settings.c (revision 13558)
+++ apps/settings.c (working copy)
@@ -515,13 +515,15 @@
return true;
}
#ifndef HAVE_RTC_RAM
-static bool flush_global_status_callback(void)
+static bool flush_global_status_callback(void* data)
{
+ (void)data;
return write_nvram_data(nvram_buffer,NVRAM_BLOCK_SIZE);
}
#endif
-static bool flush_config_block_callback(void)
+static bool flush_config_block_callback(void* data)
{
+ (void)data;
bool r1, r2;
r1 = write_nvram_data(nvram_buffer,NVRAM_BLOCK_SIZE);
r2 = settings_write_config(CONFIGFILE, SETTINGS_SAVE_CHANGED);
Index: apps/playback.c
===================================================================
--- apps/playback.c (revision 13558)
+++ apps/playback.c (working copy)
@@ -3538,8 +3538,9 @@
#if MEM > 8
/* we dont want this rebuffering on targets with little ram
because the disk may never spin down */
-static bool ata_fillbuffer_callback(void)
+static bool ata_fillbuffer_callback(void* data)
{
+ (void)data;
queue_post(&audio_queue, Q_AUDIO_FILL_BUFFER_IF_ACTIVE_ATA, 0);
return true;
}
Index: apps/scrobbler.c
===================================================================
--- apps/scrobbler.c (revision 13558)
+++ apps/scrobbler.c (working copy)
@@ -132,8 +132,9 @@
scrobbler_fd = -1;
}
-static bool scrobbler_flush_callback(void)
+static bool scrobbler_flush_callback(void* data)
{
+ (void)data;
if (scrobbler_initialised && cache_pos)
write_cache();
return true;
Index: apps/main.c
===================================================================
--- apps/main.c (revision 13558)
+++ apps/main.c (working copy)
@@ -19,7 +19,7 @@
#include "config.h"
#include "ata.h"
-#include "ata_idle_notify.h"
+#include "event_callback.h"
#include "disk.h"
#include "fat.h"
#include "lcd.h"
@@ -259,6 +259,7 @@
#ifdef DEBUG
debug_init();
#endif
+ event_callback_init();
/* Must be done before any code uses the multi-screen APi */
screen_access_init();
gui_syncstatusbar_init(&statusbars);
@@ -399,7 +400,7 @@
}
#endif
- ata_idle_notify_init();
+ event_callback_init();
rc = ata_init();
if(rc)
{
Index: firmware/export/ata_idle_notify.h
===================================================================
--- firmware/export/ata_idle_notify.h (revision 13558)
+++ firmware/export/ata_idle_notify.h (working copy)
@@ -20,6 +20,7 @@
#define __ATACALLBACK_H__
#include <stdbool.h>
+#include "event_callback.h"
#if 0
NOTE: ata_idle_nofity usage notes..
@@ -34,21 +35,22 @@
5) Dont Panic!
#endif
-#define USING_ATA_CALLBACK !defined(SIMULATOR) \
- && !defined(HAVE_FLASH_DISK)
+#if !defined(SIMULATOR) && !defined(HAVE_FLASH_DISK)
-#define MAX_ATA_CALLBACKS 5
-typedef bool (*ata_idle_notify)(void);
-
-extern bool register_ata_idle_func(ata_idle_notify function);
-#if USING_ATA_CALLBACK
-extern void ata_idle_notify_init(void);
-extern void unregister_ata_idle_func(ata_idle_notify function, bool run);
-extern bool call_ata_idle_notifys(bool force);
+#define register_ata_idle_func(f) \
+ register_event_callback(EID_ATA_DISK_SPINUP, (event_callback)f)
+
+#define unregister_ata_idle_func(f,r) \
+ unregister_event_callback(EID_ATA_DISK_SPINUP, (event_callback)f, r)
+
+#define call_ata_idle_notifys(f) \
+ call_event_callbacks(EID_ATA_DISK_SPINUP, (event_callback)f)
+
#else
+#define register_ata_idle_func(f) (f(NULL)?true:true) /* always pretend \
+ it was registered */
#define unregister_ata_idle_func(f,r)
#define call_ata_idle_notifys(f)
-#define ata_idle_notify_init(s)
#endif
#endif /* __ATACALLBACK_H__ */
Index: firmware/export/event_callback.h
===================================================================
--- firmware/export/event_callback.h (revision 0)
+++ firmware/export/event_callback.h (revision 0)
@@ -0,0 +1,57 @@
+/***************************************************************************
+ * __________ __ ___.
+ * Open \______ \ ____ ____ | | _\_ |__ _______ ___
+ * Source | _// _ \_/ ___\| |/ /| __ \ / _ \ \/ /
+ * Jukebox | | ( <_> ) \___| < | \_\ ( <_> > < <
+ * Firmware |____|_ /\____/ \___ >__|_ \|___ /\____/__/\_ \
+ * \/ \/ \/ \/ \/
+ * $Id: ata_idle_notify.h 11599 2006-11-26 09:53:42Z jdgordon $
+ *
+ * Copyright (C) 2006 Jonathan Gordon
+ *
+ * All files in this archive are subject to the GNU General Public License.
+ * See the file COPYING in the source tree root for full license agreement.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ ****************************************************************************/
+#ifndef __EVENTCALLBACKS_H__
+#define __EVENTCALLBACKS_H__
+
+#include <stdbool.h>
+
+#if 0
+ NOTE: Usage notes..
+
+ 1) Do not call cany yielding functions in the callback
+ 2) Do not call ata_sleep in the callbacks
+#endif
+
+typedef bool (*event_callback)(void* data);
+
+enum event_id {
+ EID_ATA_DISK_SPINUP, /* called from ata thread, data == NULL */
+ EID_PLAYBACK_STARTED, /* playback thread, data = ? */
+ EID_PLAYBACK_STOPPED, /* playback thread, data = ? */
+ EID_TRACK_CHANGED, /* playback thread, data = ? */
+ EID_TRACK_BUFFERED, /* playback thread, data = ? */
+ EID_TRACK_UNBUFFERED, /* playback thread, data = ? */
+};
+
+/* event - EID_ number this callback is called for
+ funtion - funtion to call
+ */
+extern bool register_event_callback(enum event_id event,
+ event_callback function);
+/* run - call the callback before removing
+ it from the list, data will always be NULL */
+extern void unregister_event_callback(enum event_id event,
+ event_callback function, bool run);
+/* call all the registered events for a event_id */
+extern bool call_event_callbacks(enum event_id event, void* data);
+
+
+extern void event_callback_init(void); /* call from main() */
+
+#endif /* __EVENTCALLBACKS_H__ */
Index: firmware/SOURCES
===================================================================
--- firmware/SOURCES (revision 13558)
+++ firmware/SOURCES (working copy)
@@ -1,6 +1,6 @@
-ata_idle_notify.c
backlight.c
buffer.c
+event_callback.c
id3.c
powermgmt.c
system.c
Index: firmware/ata_idle_notify.c
===================================================================
--- firmware/ata_idle_notify.c (revision 13558)
+++ firmware/ata_idle_notify.c (working copy)
@@ -1,105 +0,0 @@
-/***************************************************************************
- * __________ __ ___.
- * Open \______ \ ____ ____ | | _\_ |__ _______ ___
- * Source | _// _ \_/ ___\| |/ /| __ \ / _ \ \/ /
- * Jukebox | | ( <_> ) \___| < | \_\ ( <_> > < <
- * Firmware |____|_ /\____/ \___ >__|_ \|___ /\____/__/\_ \
- * \/ \/ \/ \/ \/
- * $Id$
- *
- * Copyright (C) 2006 Jonathan Gordon
- *
- * All files in this archive are subject to the GNU General Public License.
- * See the file COPYING in the source tree root for full license agreement.
- *
- * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
- * KIND, either express or implied.
- *
- ****************************************************************************/
-#include <stdbool.h>
-#include "system.h"
-#include "ata.h"
-#include "ata_idle_notify.h"
-#include "kernel.h"
-#include "string.h"
-
-#if USING_ATA_CALLBACK
-static ata_idle_notify ata_idle_notify_funcs[MAX_ATA_CALLBACKS];
-static int ata_callback_count = 0;
-#endif
-
-
-bool register_ata_idle_func(ata_idle_notify function)
-{
-#if USING_ATA_CALLBACK
- int i;
- if (ata_callback_count >= MAX_ATA_CALLBACKS)
- return false;
- for (i=0; i<MAX_ATA_CALLBACKS; i++)
- {
- if (ata_idle_notify_funcs[i] == NULL)
- {
- ata_idle_notify_funcs[i] = function;
- ata_callback_count++;
- return true;
- }
- else if (ata_idle_notify_funcs[i] == function)
- return true;
- }
- return false;
-#else
- function(); /* just call the function now */
-/* this _may_ cause problems later if the calling function
- sets a variable expecting the callback to unset it, because
- the callback will be run before this function exits, so before the var is set */
- return true;
-#endif
-}
-
-#if USING_ATA_CALLBACK
-void unregister_ata_idle_func(ata_idle_notify func, bool run)
-{
- int i;
- for (i=0; i<MAX_ATA_CALLBACKS; i++)
- {
- if (ata_idle_notify_funcs[i] == func)
- {
- ata_idle_notify_funcs[i] = NULL;
- ata_callback_count--;
- if (run) func();
- }
- }
- return;
-}
-
-bool call_ata_idle_notifys(bool force)
-{
- int i;
- static int lock_until = 0;
- ata_idle_notify function;
- if (!force)
- {
- if (TIME_BEFORE(current_tick,lock_until) )
- return false;
- }
- lock_until = current_tick + 30*HZ;
-
- for (i = 0; i < MAX_ATA_CALLBACKS; i++)
- {
- if (ata_idle_notify_funcs[i])
- {
- function = ata_idle_notify_funcs[i];
- ata_idle_notify_funcs[i] = NULL;
- function();
- ata_callback_count--;
- }
- }
- return true;
-}
-
-void ata_idle_notify_init(void)
-{
- ata_callback_count = 0;
- memset(ata_idle_notify_funcs, 0, sizeof(ata_idle_notify_funcs));
-}
-#endif
Index: firmware/event_callback.c
===================================================================
--- firmware/event_callback.c (revision 0)
+++ firmware/event_callback.c (revision 0)
@@ -0,0 +1,94 @@
+/***************************************************************************
+ * __________ __ ___.
+ * Open \______ \ ____ ____ | | _\_ |__ _______ ___
+ * Source | _// _ \_/ ___\| |/ /| __ \ / _ \ \/ /
+ * Jukebox | | ( <_> ) \___| < | \_\ ( <_> > < <
+ * Firmware |____|_ /\____/ \___ >__|_ \|___ /\____/__/\_ \
+ * \/ \/ \/ \/ \/
+ * $Id: ata_idle_notify.c 11674 2006-12-06 12:11:57Z jdgordon $
+ *
+ * Copyright (C) 2006 Jonathan Gordon
+ *
+ * All files in this archive are subject to the GNU General Public License.
+ * See the file COPYING in the source tree root for full license agreement.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ ****************************************************************************/
+#include <stdbool.h>
+#include "system.h"
+#include "ata.h"
+#include "event_callback.h"
+#include "kernel.h"
+#include "string.h"
+
+struct callback_info {
+ event_callback function;
+ enum event_id event;
+};
+#define MAX_CALLBACKS 16
+static struct callback_info callbacks[MAX_CALLBACKS];
+static int callback_count = 0;
+
+bool register_event_callback(enum event_id event,
+ event_callback function)
+{
+ int i;
+ if (callback_count >= MAX_CALLBACKS)
+ /* possibly panic() here */
+ return false;
+
+ for (i=0; i<MAX_CALLBACKS; i++)
+ {
+ if (callbacks[i].function == NULL)
+ {
+ callbacks[i].function = function;
+ callbacks[i].event = event;
+ callback_count++;
+ return true;
+ }
+ else if ((callbacks[i].function == function) &&
+ (callbacks[i].event == event))
+ return true;
+ }
+ return false;
+}
+
+void unregister_event_callback(enum event_id event,
+ event_callback function, bool run)
+{
+ int i;
+ for (i=0; i<MAX_CALLBACKS; i++)
+ {
+ if ((callbacks[i].function == function) &&
+ (callbacks[i].event == event))
+ {
+ if (run)
+ callbacks[i].function(NULL);
+ callbacks[i].function = NULL;
+ callback_count--;
+ break;
+ }
+ }
+ return;
+}
+bool call_event_callbacks(enum event_id event, void* data)
+{
+ int i;
+ for (i=0; i<MAX_CALLBACKS; i++)
+ {
+ if ((callbacks[i].function) &&
+ (callbacks[i].event == event))
+ {
+ callbacks[i].function(data);
+ }
+ }
+ return true;
+}
+
+void event_callback_init(void)
+{
+ callback_count = 0;
+ memset(callbacks, 0, sizeof(callbacks));
+}
