[PATCH] add option to resume playback on startup

[Johannes Weißl]

If resume=true, cmus will start playing right from the position it had
when exiting. Information is saved to $CMUS_HOME/resume.
---
Doc/cmus.txt | 3 +
TODO | 1 -
command_mode.c | 2 +-
lib.c | 8 ++--
lib.h | 3 +
misc.c | 26 +++++++++++
misc.h | 1 +
options.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
options.h | 6 +++
player.c | 11 ++++-
player.h | 6 ++-
server.c | 3 +-
ui_curses.c | 5 ++
13 files changed, 193 insertions(+), 11 deletions(-)

diff --git a/Doc/cmus.txt b/Doc/cmus.txt
index 83a0b78..38409b3 100644
--- a/Doc/cmus.txt
+++ b/Doc/cmus.txt
@@ -886,6 +886,9 @@ replaygain_limit (true)
replaygain_preamp (6.0)
Replay gain preamplification in decibels.

+resume (false)
+ Resume playback on startup.
+
show_hidden (false)
Display hidden files in browser.

diff --git a/TODO b/TODO
index be4186a..572d5d3 100644
--- a/TODO
+++ b/TODO
@@ -3,6 +3,5 @@
- cuesheet (.flac + .cue) support
- CDDA support
- Last.FM support
-- option to resume playback on startup
- different random modes (albums/artist/etc.)
- playlist tracks grouping (a la foobar2000)
diff --git a/command_mode.c b/command_mode.c
index 62a1a0a..f6d93fb 100644
--- a/command_mode.c
+++ b/command_mode.c
@@ -575,7 +575,7 @@ inside:
}

if (!*arg) {
- player_seek(seek, relative);
+ player_seek(seek, relative, 1);
return;
}
err:
diff --git a/lib.c b/lib.c
index 1367731..28ae6d8 100644
--- a/lib.c
+++ b/lib.c
@@ -329,7 +329,7 @@ void lib_init(void)
srand(time(NULL));
}

-static struct track_info *lib_set_track(struct tree_track *track)
+struct track_info *lib_set_track(struct tree_track *track)
{
struct track_info *ti = NULL;

@@ -415,7 +415,7 @@ static void hash_add_to_views(void)
}
}

-static struct tree_track *find_tree_track(struct track_info *ti)
+struct tree_track *lib_find_track(struct track_info *ti)
{
struct simple_track *track;

@@ -430,7 +430,7 @@ static struct tree_track *find_tree_track(struct track_info *ti)

static void restore_cur_track(struct track_info *ti)
{
- struct tree_track *tt = find_tree_track(ti);
+ struct tree_track *tt = lib_find_track(ti);
if (tt)
lib_cur_track = tt;
}
@@ -527,7 +527,7 @@ static void store_sel_track(void)
static void restore_sel_track(void)
{
if (sel_track_ti) {
- struct tree_track *tt = find_tree_track(sel_track_ti);
+ struct tree_track *tt = lib_find_track(sel_track_ti);
if (tt) {
set_sel_track(tt);
track_info_unref(sel_track_ti);
diff --git a/lib.h b/lib.h
index 1a6d38b..3d6fe16 100644
--- a/lib.h
+++ b/lib.h
@@ -104,6 +104,9 @@ void lib_set_view(int view);
int lib_for_each(int (*cb)(void *data, struct track_info *ti), void *data);
int lib_for_each_filtered(int (*cb)(void *data, struct track_info *ti), void *data);

+struct tree_track *lib_find_track(struct track_info *ti);
+struct track_info *lib_set_track(struct tree_track *track);
+
struct tree_track *tree_get_selected(void);
struct track_info *tree_set_selected(void);
void tree_sort_artists(void);
diff --git a/misc.c b/misc.c
index 6e77fd5..1bf64a6 100644
--- a/misc.c
+++ b/misc.c
@@ -109,6 +109,32 @@ const char *escape(const char *str)
return buf;
}

+const char *unescape(const char *str)
+{
+ static char *buf = NULL;
+ static size_t alloc = 0;
+ size_t need = strlen(str) + 1;
+ int do_escape = 0;
+ int s, d;
+
+ if (need > alloc) {
+ alloc = (need + 16) & ~(16 - 1);
+ buf = xrealloc(buf, alloc);
+ }
+
+ d = 0;
+ for (s = 0; str[s]; s++) {
+ if (!do_escape && str[s] == '\\')
+ do_escape = 1;
+ else {
+ buf[d++] = (do_escape && str[s] == 'n') ? '\n' : str[s];
+ do_escape = 0;
+ }
+ }
+ buf[d] = 0;
+ return buf;
+}
+
static int dir_exists(const char *dirname)
{
DIR *dir;
diff --git a/misc.h b/misc.h
index f12d857..0e0a80b 100644
--- a/misc.h
+++ b/misc.h
@@ -28,5 +28,6 @@ char **get_words(const char *text);
int strptrcmp(const void *a, const void *b);
int misc_init(void);
const char *escape(const char *str);
+const char *unescape(const char *str);

#endif
diff --git a/options.c b/options.c
index 75eca52..80b13bf 100644
--- a/options.c
+++ b/options.c
@@ -23,6 +23,8 @@
#include "output.h"
#include "config/datadir.h"
#include "track_info.h"
+#include "cache.h"
+#include "debug.h"

#include <stdio.h>
#include <errno.h>
@@ -41,6 +43,7 @@ char *status_display_program = NULL;
char *server_password;
int auto_reshuffle = 1;
int confirm_run = 1;
+int resume_cmus = 0;
int show_hidden = 0;
int show_remaining_time = 0;
int set_term_title = 1;
@@ -677,6 +680,21 @@ static void toggle_replaygain_limit(unsigned int id)
player_set_rg_limit(replaygain_limit ^ 1);
}

+static void get_resume(unsigned int id, char *buf)
+{
+ strcpy(buf, bool_names[resume_cmus]);
+}
+
+static void set_resume(unsigned int id, const char *buf)
+{
+ parse_bool(buf, &resume_cmus);
+}
+
+static void toggle_resume(unsigned int id)
+{
+ resume_cmus ^= 1;
+}
+
static void get_show_hidden(unsigned int id, char *buf)
{
strcpy(buf, bool_names[show_hidden]);
@@ -890,6 +908,7 @@ static const struct {
DT(replaygain)
DT(replaygain_limit)
DN(replaygain_preamp)
+ DT(resume)
DT(show_hidden)
DT(show_remaining_time)
DT(set_term_title)
@@ -1107,3 +1126,113 @@ void options_exit(void)

fclose(f);
}
+
+struct resume {
+ enum player_status status;
+ char *filename;
+ long int position;
+ char *lib_filename;
+ int view;
+};
+
+static int handle_resume_line(void *data, const char *line)
+{
+ struct resume *resume = data;
+ char *cmd, *arg;
+
+ if (!parse_command(line, &cmd, &arg))
+ return 0;
+ if (!arg)
+ goto out;
+
+ if (strcmp(cmd, "status") == 0) {
+ parse_enum(arg, 0, NR_PLAYER_STATUS, player_status_names, (int *) &resume->status);
+ } else if (strcmp(cmd, "file") == 0) {
+ resume->filename = xstrdup(unescape(arg));
+ } else if (strcmp(cmd, "position") == 0) {
+ str_to_int(arg, &resume->position);
+ } else if (strcmp(cmd, "lib_file") == 0) {
+ resume->lib_filename = xstrdup(unescape(arg));
+ } else if (strcmp(cmd, "view") == 0) {
+ parse_enum(arg, 0, NR_VIEWS, view_names, &resume->view);
+ }
+
+ free(arg);
+out:
+ free(cmd);
+ return 0;
+}
+
+void resume_load(void)
+{
+ char filename[512];
+ struct track_info *ti, *old;
+ struct resume resume = { .status = PLAYER_STATUS_STOPPED, .view = -1 };
+
+ snprintf(filename, sizeof(filename), "%s/resume", cmus_config_dir);
+ if (file_for_each_line(filename, handle_resume_line, &resume) == -1) {
+ if (errno != ENOENT)
+ error_msg("loading %s: %s", filename, strerror(errno));
+ return;
+ }
+ if (resume.view >= 0 && resume.view != cur_view)
+ set_view(resume.view);
+ if (resume.lib_filename) {
+ cache_lock();
+ ti = old = cache_get_ti(resume.lib_filename);
+ cache_unlock();
+ if (ti) {
+ editable_lock();
+ lib_add_track(ti);
+ track_info_unref(ti);
+ ti = lib_set_track(lib_find_track(ti));
+ BUG_ON(ti != old);
+ track_info_unref(ti);
+ tree_sel_current();
+ sorted_sel_current();
+ editable_unlock();
+ }
+ free(resume.lib_filename);
+ }
+ if (resume.filename) {
+ cache_lock();
+ ti = cache_get_ti(resume.filename);
+ cache_unlock();
+ if (ti) {
+ player_set_file(ti);
+ if (resume.status != PLAYER_STATUS_STOPPED)
+ player_seek(resume.position, 0, resume.status == PLAYER_STATUS_PLAYING);
+ }
+ free(resume.filename);
+ }
+}
+
+void resume_exit(void)
+{
+ char filename[512];
+ struct track_info *ti;
+ FILE *f;
+
+ snprintf(filename, sizeof(filename), "%s/resume", cmus_config_dir);
+ f = fopen(filename, "w");
+ if (!f) {
+ warn_errno("creating %s", filename);
+ return;
+ }
+
+ player_info_lock();
+ fprintf(f, "status %s\n", player_status_names[player_info.status]);
+ ti = player_info.ti;
+ if (ti) {
+ fprintf(f, "file %s\n", escape(ti->filename));
+ fprintf(f, "position %d\n", player_info.pos);
+ }
+ player_info_unlock();
+ if (lib_cur_track) {
+ ti = tree_track_info(lib_cur_track);
+ fprintf(f, "lib_file %s\n", escape(ti->filename));
+ }
+ fprintf(f, "view %s\n", view_names[cur_view]);
+
+ fclose(f);
+}
diff --git a/options.h b/options.h
index 53e3dbb..49125d7 100644
--- a/options.h
+++ b/options.h
@@ -86,6 +86,7 @@ extern char *status_display_program;
extern char *server_password;
extern int auto_reshuffle;
extern int confirm_run;
+extern int resume_cmus;
extern int show_hidden;
extern int show_remaining_time;
extern int set_term_title;
@@ -131,6 +132,11 @@ int source_file(const char *filename);
/* save options */
void options_exit(void);

+/* load resume file */
+void resume_load(void);
+/* save resume file */
+void resume_exit(void);
+
void option_add(const char *name, unsigned int id, opt_get_cb get,
opt_set_cb set, opt_toggle_cb toggle);
struct cmus_opt *option_find(const char *name);
diff --git a/player.c b/player.c
index 573a49a..2d73f91 100644
--- a/player.c
+++ b/player.c
@@ -36,6 +36,8 @@
#include <stdarg.h>
#include <math.h>

+const char * const player_status_names[] = { "stopped", "playing", "paused", NULL };
+
enum producer_status {
PS_UNLOADED,
PS_STOPPED,
@@ -1172,10 +1174,12 @@ out:
player_unlock();
}

-void player_seek(double offset, int relative)
+void player_seek(double offset, int relative, int start_playing)
{
+ int stopped = 0;
player_lock();
if (consumer_status == CS_STOPPED) {
+ stopped = 1;
__producer_play();
if (producer_status == PS_PLAYING) {
__consumer_play();
@@ -1238,6 +1242,11 @@ void player_seek(double offset, int relative)
consumer_pos = new_pos * buffer_second_size();
scale_pos = consumer_pos;
__consumer_position_update();
+ if (stopped && !start_playing) {
+ __producer_pause();
+ __consumer_pause();
+ __player_status_changed();
+ }
} else {
d_print("error: ip_seek returned %d\n", rc);
}
diff --git a/player.h b/player.h
index ca57a5e..ef12e7b 100644
--- a/player.h
+++ b/player.h
@@ -35,10 +35,12 @@ enum {
PLAYER_ERROR_NOT_SUPPORTED
};

+extern const char * const player_status_names[];
enum player_status {
PLAYER_STATUS_STOPPED,
PLAYER_STATUS_PLAYING,
- PLAYER_STATUS_PAUSED
+ PLAYER_STATUS_PAUSED,
+ NR_PLAYER_STATUS
};

enum replaygain {
@@ -101,7 +103,7 @@ void player_play_file(struct track_info *ti);
void player_play(void);
void player_stop(void);
void player_pause(void);
-void player_seek(double offset, int relative);
+void player_seek(double offset, int relative, int start_playing);
void player_set_op(const char *name);
void player_set_buffer_chunks(unsigned int nr_chunks);
int player_get_buffer_chunks(void);
diff --git a/server.c b/server.c
index 797086b..d65fb82 100644
--- a/server.c
+++ b/server.c
@@ -61,7 +61,6 @@ static union {

static int cmd_status(struct client *client)
{
- const char *status[] = { "stopped", "playing", "paused" };
const char *export_options[] = {
"aaa_mode",
"continue",
@@ -84,7 +83,7 @@ static int cmd_status(struct client *client)
int i, ret;

player_info_lock();
- gbuf_addf(&buf, "status %s\n", status[player_info.status]);
+ gbuf_addf(&buf, "status %s\n", player_status_names[player_info.status]);
ti = player_info.ti;
if (ti) {
gbuf_addf(&buf, "file %s\n", escape(ti->filename));
diff --git a/ui_curses.c b/ui_curses.c
index 50a0df5..f0efd9f 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -2205,6 +2205,9 @@ static void init_all(void)
if (!soft_vol)
mixer_open();

+ if (resume_cmus)
+ resume_load();
+
lib_autosave_filename = xstrjoin(cmus_config_dir, "/lib.pl");
pl_autosave_filename = xstrjoin(cmus_config_dir, "/playlist.pl");
pl_filename = xstrdup(pl_autosave_filename);
@@ -2231,6 +2234,8 @@ static void exit_all(void)
{
endwin();

+ if (resume_cmus)
+ resume_exit();
options_exit();

server_exit();

[Jason Woofenden]

Awesome feature!

Fun for music, a must-have for audio books.

Works great for me (debian unstable, x86, pulseaudio)

It even works when paused (it resumes still paused, at the correct
track/position.)

Makes me wish that I could have it auto-restore live-filter too :)

Thank you! - Jason

[Johannes Weißl]

Hello Jason,

Post by Jason Woofenden
Awesome feature!
Fun for music, a must-have for audio books.

Yes, definitely! I was hearing audio plays lately, and it is really
useful. Great that you like it!

Post by Jason Woofenden
It even works when paused (it resumes still paused, at the correct
track/position.)

Yes... I had to tune the player_seek() function for that (additional
parameter). I was asking myself, if this should be the default behaviour
when seeking (:seek 1:20 on a stopped track should stay paused). But
then, changing accustomed behaviour may be a bad idea!

Post by Jason Woofenden
Makes me wish that I could have it auto-restore live-filter too :)

Nothing easier than that :-). Maybe also restore the queue when
resume=true? Are there other things worthy of restoring?

There is one problem though: it is not easily possible to restore the
current selected track in the playlist. The playlist add job is run
asynchronously, and could in theory take any amount of time. And since
the order can be random (if pl_sort is unset), we can't add the selected
track right at the beginning as with the library :-(.

One other question:
Could we change the behaviour of job.c to a stack instead of a queue?
So new :add requests will have priority over old requests. Use case:
You add your huge library, which is not in cache yet. But you want to
listen to a certain song right away, so you add it to the playlist. But
cmus will wait till the whole library has been added and only then add
the one song... What do you think?


P.S.: I have updated the next branch with all the new patches!


Greetings,
Johannes

[Jason Woofenden]

Post by Johannes Weißl

Post by Jason Woofenden
It even works when paused (it resumes still paused, at the correct
track/position.)

Yes... I had to tune the player_seek() function for that (additional
parameter). I was asking myself, if this should be the default behaviour
when seeking (:seek 1:20 on a stopped track should stay paused). But
then, changing accustomed behaviour may be a bad idea!

It's a tough call. mplayer unpauses for seeks. I think that's fine.
What bothers me about mplayer is that it unpauses when you pop out
of fullscreen mode. I suspect mplayer may do it not because they
decided it was good behavior, but because it doesn't know how to
redraw the screen without playing.

Post by Johannes Weißl

Post by Jason Woofenden
Makes me wish that I could have it auto-restore live-filter too :)

Nothing easier than that :-). Maybe also restore the queue when
resume=true? Are there other things worthy of restoring?

Oooh! I'd love my queue to persist across runs.

Post by Johannes Weißl
There is one problem though: it is not easily possible to restore the
current selected track in the playlist. The playlist add job is run
asynchronously, and could in theory take any amount of time. And since
the order can be random (if pl_sort is unset), we can't add the selected
track right at the beginning as with the library :-(.

Tricky!

Post by Johannes Weißl
Could we change the behaviour of job.c to a stack instead of a queue?
You add your huge library, which is not in cache yet. But you want to
listen to a certain song right away, so you add it to the playlist. But
cmus will wait till the whole library has been added and only then add
the one song... What do you think?

Sounds good to me. I'm not familiar with the code. So when it's
recursively adding a directory, it queues the tracks inside? If so
it sounds like it would be easy to get newer :add commands to be
processed midway through a directory import.

Post by Johannes Weißl
P.S.: I have updated the next branch with all the new patches!

Yeah :) that's what I'm running.

Take care, - Jason

[gt]

First of all thanks to Johannes for this really useful patch. A lot of
people will love it, as is evident from our small mailing list.

Post by Jason Woofenden
It's a tough call. mplayer unpauses for seeks. I think that's fine.
What bothers me about mplayer is that it unpauses when you pop out
of fullscreen mode. I suspect mplayer may do it not because they
decided it was good behavior, but because it doesn't know how to
redraw the screen without playing.

You should take a look at mplayer2. It's a fork of mplayer, and it
remains paused when doing things like seek, resize etc.

http://www.mplayer2.org/comparison.html

[Johannes Weißl]

Post by Jason Woofenden

Post by Johannes Weißl
Nothing easier than that :-). Maybe also restore the queue when
resume=true? Are there other things worthy of restoring?

Oooh! I'd love my queue to persist across runs.

Done :-)

Post by Jason Woofenden

Post by Johannes Weißl
Could we change the behaviour of job.c to a stack instead of a queue?
You add your huge library, which is not in cache yet. But you want to
listen to a certain song right away, so you add it to the playlist. But
cmus will wait till the whole library has been added and only then add
the one song... What do you think?

Sounds good to me. I'm not familiar with the code. So when it's
recursively adding a directory, it queues the tracks inside? If so
it sounds like it would be easy to get newer :add commands to be
processed midway through a directory import.

Yes, basically this is how it's done. But the recursively found
subdirectories are not queued into the job queue, but processed
directly. So this is what I want to change. I will look into it and post
results!


Johannes

[a.l.e]

hi

Post by Johannes Weißl
Maybe also restore the queue when
resume=true? Are there other things worthy of restoring?

yes, good idea! this is something i have been waiting for long.

Post by Johannes Weißl
Could we change the behaviour of job.c to a stack instead of a queue?
You add your huge library, which is not in cache yet. But you want to
listen to a certain song right away, so you add it to the playlist. But
cmus will wait till the whole library has been added and only then add
the one song... What do you think?

please don't.

i always use the queue and i mostly want to add the next track at the end.
if i want to listen to a single track right away (it does happen from
time to time) i just type "E" (i hope that there is a ":" command which
does the same...)

well, technically, i never use :add, so i shouldn't really care... but i
guess that "e" and :add should do the same... or not?

ciao
a.l.e

[Jason Woofenden]

a.l.e,

:add adds files to the library. He's not talking about changing how
things are added to the queue.

I believe "a" is bound to :add for the file/directory browser view.

Take care, - Jason

[a.l.e]

hi

Post by Jason Woofenden
:add adds files to the library. He's not talking about changing how
things are added to the queue.
I believe "a" is bound to :add for the file/directory browser view.

ah, ok! thanks for the explaination...

i can go on sleeping, then!

ciao
a..e

[Johannes Weißl]

Post by a.l.e

Post by Johannes Weißl
Maybe also restore the queue when
resume=true? Are there other things worthy of restoring?

yes, good idea! this is something i have been waiting for long.

Ok, done!

Post by a.l.e

Post by Johannes Weißl
Could we change the behaviour of job.c to a stack instead of a queue?
You add your huge library, which is not in cache yet. But you want to
listen to a certain song right away, so you add it to the playlist. But
cmus will wait till the whole library has been added and only then add
the one song... What do you think?

please don't.
i always use the queue and i mostly want to add the next track at the end.
if i want to listen to a single track right away (it does happen
from time to time) i just type "E" (i hope that there is a ":"
command which does the same...)
well, technically, i never use :add, so i shouldn't really care...
but i guess that "e" and :add should do the same... or not?

Hmm, I think there is a misunderstanding! The proposed change doesn't
affect the position of the tracks in library/playlist/queue at all. It
just makes the worker job scheduler more "fair": If you add 30000 tracks
to the library, and right after that, 2 files to the queue, cmus will
first add all 30000 tracks (which can take a long time), and after
that add the two tracks to the queue. I would like cmus to e.g. add 32
of the 30000 tracks, then add the two queue tracks, and then continue
with the rest.

Of course the order of the queue/playlist would not be affected: if you append
100 tracks to the queue, and then two more, they will still be after the
100.

Are you ok with this change or do you see problems?


Johannes

[a.l.e]

hallo johannes,

Post by Johannes Weißl
Of course the order of the queue/playlist would not be affected: if
you append 100 tracks to the queue, and then two more, they will
still be after the 100.
Are you ok with this change or do you see problems?

no problem at all: i normally append one cd at a time :-)

ciao && thanks for all the good work on cmus!
a.l.e

[Gregory Petrosyan]

Post by Johannes Weißl
Could we change the behaviour of job.c to a stack instead of a queue?
You add your huge library, which is not in cache yet. But you want to
listen to a certain song right away, so you add it to the playlist. But
cmus will wait till the whole library has been added and only then add
the one song... What do you think?

This sounds reasonable. Maybe it is even possible to add all 'spawned' :add
requests to the end of queue, and all 'manual' :add requests to the front? Not
sure if this twist is really needed, though.

Post by Johannes Weißl
P.S.: I have updated the next branch with all the new patches!

Or you can grab all of them from -pu now :-)

Gregory

[Johannes Weißl]

Hello Gregory,

Post by Gregory Petrosyan

Post by Johannes Weißl
Could we change the behaviour of job.c to a stack instead of a queue?
You add your huge library, which is not in cache yet. But you want to
listen to a certain song right away, so you add it to the playlist. But
cmus will wait till the whole library has been added and only then add
the one song... What do you think?

This sounds reasonable. Maybe it is even possible to add all 'spawned' :add
requests to the end of queue, and all 'manual' :add requests to the front? Not
sure if this twist is really needed, though.

I had the same idea! I think this is a pretty good strategy. Also I
replaced the single queue with one for every job. What do you think?
(patch in next mail)


Johannes

[Johannes Weißl]

* Three different queues (play_queue, playlist, library):
This way it is possible to prioritize play_queue and playlist jobs
over library jobs (because they are smaller and probably more
important for next song). Also the code gets cleaner!
* When adding directories recursively, each subdirectory becomes a new
job. This allows insertion of new jobs (see below). It also means
directories are now processed breadth-first, which might improve
cache locality.
* A new :add to library prepends a job. The order is not important for
the library, and this way it is possible to insert jobs before a
currently running job.
---
cmus.c | 17 ++++++-
cmus.h | 20 ++++----
command_mode.c | 2 +-
job.c | 2 +-
ui_curses.c | 2 +-
worker.c | 135 ++++++++++++++++++++++++++++++++++++++++----------------
worker.h | 37 ++++++++++-----
7 files changed, 150 insertions(+), 65 deletions(-)

diff --git a/cmus.c b/cmus.c
index 6d09165..b8500f6 100644
--- a/cmus.c
+++ b/cmus.c
@@ -46,7 +46,7 @@ int cmus_init(void)

void cmus_exit(void)
{
- worker_remove_jobs(JOB_TYPE_ANY);
+ worker_remove_all_jobs();
worker_exit();
if (cache_close())
d_print("error: %s\n", strerror(errno));
@@ -148,7 +148,20 @@ void cmus_add(add_ti_cb add, const char *name, enum file_type ft, int jt)
data->add = add;
data->name = xstrdup(name);
data->type = ft;
- worker_add_job(jt, do_add_job, free_add_job, data);
+ if (jt == JOB_TYPE_LIB)
+ worker_prepend_job(jt, do_add_job, free_add_job, data);
+ else
+ worker_add_job(jt, do_add_job, free_add_job, data);
+}
+
+void cmus_amend_add(add_ti_cb add, const char *name, enum file_type ft)
+{
+ struct add_data *data = xnew(struct add_data, 1);
+
+ data->add = add;
+ data->name = xstrdup(name);
+ data->type = ft;
+ worker_amend_current_job(data);
}

static int save_ext_playlist_cb(void *data, struct track_info *ti)
diff --git a/cmus.h b/cmus.h
index a0227e1..2bf5899 100644
--- a/cmus.h
+++ b/cmus.h
@@ -3,16 +3,6 @@

#include "track_info.h"

-/*
- * these types are only used to determine what jobs we should cancel.
- * for example ":load" cancels jobs for the current view before loading
- * new playlist.
- */
-
-#define JOB_TYPE_LIB 1
-#define JOB_TYPE_PL 2
-#define JOB_TYPE_QUEUE 3
-
enum file_type {
/* not found, device file... */
FILE_TYPE_INVALID,
@@ -55,6 +45,16 @@ enum file_type cmus_detect_ft(const char *name, char **ret);
*/
void cmus_add(add_ti_cb, const char *name, enum file_type ft, int jt);

+/* add to a currently running cmus_add process
+ *
+ * @add callback that does the actual adding
+ * @name playlist, directory, file, URL
+ * @ft detected FILE_TYPE_*
+ *
+ * returns immediately, actual work is done in the worker thread.
+ */
+void cmus_amend_add(add_ti_cb add, const char *name, enum file_type ft);
+
int cmus_save(for_each_ti_cb for_each_ti, const char *filename);
int cmus_save_ext(for_each_ti_cb for_each_ti, const char *filename);

diff --git a/command_mode.c b/command_mode.c
index 62a1a0a..67c5192 100644
--- a/command_mode.c
+++ b/command_mode.c
@@ -783,7 +783,7 @@ err:
static void cmd_quit(char *arg)
{
int flag = parse_flags((const char **)&arg, "i");
- if (!worker_has_job(JOB_TYPE_ANY)) {
+ if (!worker_has_any_jobs()) {
if (flag != 'i' || yes_no_query("Quit cmus? [y/N]"))
cmus_running = 0;
} else {
diff --git a/job.c b/job.c
index a1b71f1..ba5eaeb 100644
--- a/job.c
+++ b/job.c
@@ -147,7 +147,7 @@ static void add_dir(const char *dirname, const char *root)

memcpy(dir.path + dir.len, ents[i]->name, len + 1);
if (S_ISDIR(ents[i]->mode)) {
- add_dir(dir.path, root);
+ cmus_amend_add(jd->add, dir.path, FILE_TYPE_DIR);
} else {
add_file(dir.path);
}
diff --git a/ui_curses.c b/ui_curses.c
index 50a0df5..bb8b7c0 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -1988,7 +1988,7 @@ static void main_loop(void)
}
player_info_unlock();

- if (!tv.tv_usec && worker_has_job(JOB_TYPE_ANY)) {
+ if (!tv.tv_usec && worker_has_any_jobs()) {
// playlist is loading. screen needs to be updated
tv.tv_usec = 250e3;
}
diff --git a/worker.c b/worker.c
index b936f3a..9111c82 100644
--- a/worker.c
+++ b/worker.c
@@ -7,21 +7,23 @@
#include <stdlib.h>
#include <pthread.h>

+#define JOB_TYPE_NONE (NR_JOB_TYPE+1)
+#define JOB_TYPE_ANY (NR_JOB_TYPE+2)
+
struct worker_job {
struct list_head node;
- /* >0, 0 is 'any' */
- int type;
+ enum job_type type;
void (*job_cb)(void *data);
void (*free_cb)(void *data);
void *data;
};

-static LIST_HEAD(worker_job_head);
+static struct list_head worker_job_heads[NR_JOB_TYPE];
static pthread_mutex_t worker_mutex = CMUS_MUTEX_INITIALIZER;
static pthread_cond_t worker_cond = PTHREAD_COND_INITIALIZER;
static pthread_t worker_thread;
static int running = 1;
-static int cancel_type = JOB_TYPE_NONE;
+static enum job_type cancel_type = JOB_TYPE_NONE;

/*
* - only worker thread modifies this
@@ -33,11 +35,23 @@ static struct worker_job *cur_job = NULL;
#define worker_lock() cmus_mutex_lock(&worker_mutex)
#define worker_unlock() cmus_mutex_unlock(&worker_mutex)

+static inline enum job_type next_job_type(void)
+{
+ int i;
+ for (i = 0; i < NR_JOB_TYPE; i++) {
+ if (!list_empty(&worker_job_heads[i]))
+ return i;
+ }
+ return JOB_TYPE_NONE;
+}
+
static void *worker_loop(void *arg)
{
worker_lock();
while (1) {
- if (list_empty(&worker_job_head)) {
+ enum job_type next = next_job_type();
+
+ if (next == JOB_TYPE_NONE) {
int rc;

if (!running)
@@ -47,7 +61,7 @@ static void *worker_loop(void *arg)
if (rc)
d_print("pthread_cond_wait: %s\n", strerror(rc));
} else {
- struct list_head *item = worker_job_head.next;
+ struct list_head *item = worker_job_heads[next].next;
uint64_t t;

list_del(item);
@@ -74,7 +88,10 @@ static void *worker_loop(void *arg)

void worker_init(void)
{
- int rc = pthread_create(&worker_thread, NULL, worker_loop, NULL);
+ int i, rc = pthread_create(&worker_thread, NULL, worker_loop, NULL);
+
+ for (i = 0; i < NR_JOB_TYPE; i++)
+ list_init(&worker_job_heads[i]);

BUG_ON(rc);
}
@@ -89,8 +106,8 @@ void worker_exit(void)
pthread_join(worker_thread, NULL);
}

-void worker_add_job(int type, void (*job_cb)(void *data),
- void (*free_cb)(void *data), void *data)
+static void add_job(enum job_type type, void (*job_cb)(void *data),
+ void (*free_cb)(void *data), void *data, int prepend)
{
struct worker_job *job;

@@ -101,56 +118,98 @@ void worker_add_job(int type, void (*job_cb)(void *data),
job->data = data;

worker_lock();
- list_add_tail(&job->node, &worker_job_head);
+ if (prepend)
+ list_add(&job->node, &worker_job_heads[type]);
+ else
+ list_add_tail(&job->node, &worker_job_heads[type]);
pthread_cond_signal(&worker_cond);
worker_unlock();
}

-void worker_remove_jobs(int type)
+void worker_add_job(enum job_type type, void (*job_cb)(void *data),
+ void (*free_cb)(void *data), void *data)
{
- struct list_head *item;
+ add_job(type, job_cb, free_cb, data, 0);
+}

- worker_lock();
- cancel_type = type;
+void worker_prepend_job(enum job_type type, void (*job_cb)(void *data),
+ void (*free_cb)(void *data), void *data)
+{
+ add_job(type, job_cb, free_cb, data, 1);
+}

- /* remove jobs of the specified type from the queue */
- item = worker_job_head.next;
- while (item != &worker_job_head) {
- struct worker_job *job = container_of(item, struct worker_job, node);
- struct list_head *next = item->next;
+/*
+ * this is only called from the worker thread
+ * cur_job is guaranteed to be non-NULL
+ */
+void worker_amend_current_job(void *data)
+{
+ add_job(cur_job->type, cur_job->job_cb, cur_job->free_cb, data, 0);
+}

- if (type == JOB_TYPE_ANY || type == job->type) {
- list_del(&job->node);
- job->free_cb(job->data);
- free(job);
- }
- item = next;
+/* remove jobs of the specified type from the queue */
+static inline void remove_jobs(enum job_type type)
+{
+ struct list_head *item, *tmp, *head = &worker_job_heads[type];
+
+ list_for_each_safe(item, tmp, head) {
+ struct worker_job *job = list_entry(item, struct worker_job, node);
+
+ list_del(&job->node);
+ job->free_cb(job->data);
+ free(job);
}
+}
+
+void worker_remove_jobs(enum job_type type)
+{
+ worker_lock();
+ cancel_type = type;
+
+ remove_jobs(type);

/* wait current job to finish or cancel if it's of the specified type */
- if (cur_job && (type == JOB_TYPE_ANY || type == cur_job->type))
+ if (cur_job && type == cur_job->type)
pthread_cond_wait(&worker_cond, &worker_mutex);

cancel_type = JOB_TYPE_NONE;
worker_unlock();
}

-int worker_has_job(int type)
+void worker_remove_all_jobs(void)
{
- struct worker_job *job;
- int has_job = 0;
+ int i;

worker_lock();
- list_for_each_entry(job, &worker_job_head, node) {
- if (type == JOB_TYPE_ANY || type == job->type) {
- has_job = 1;
- break;
- }
- }
- if (cur_job && (type == JOB_TYPE_ANY || type == cur_job->type))
- has_job = 1;
+ cancel_type = JOB_TYPE_ANY;
+
+ for (i = 0; i < NR_JOB_TYPE; i++)
+ remove_jobs(i);
+
+ /* wait current job to finish or cancel */
+ if (cur_job)
+ pthread_cond_wait(&worker_cond, &worker_mutex);
+
+ cancel_type = JOB_TYPE_NONE;
+ worker_unlock();
+}
+
+int worker_has_job(enum job_type type)
+{
+ int rc;
+ worker_lock();
+ rc = !list_empty(&worker_job_heads[type]) || (cur_job && type == cur_job->type);
+ worker_unlock();
+ return rc;
+}
+
+int worker_has_any_jobs(void)
+{
+ int rc;
+ worker_lock();
+ rc = cur_job || (next_job_type() != JOB_TYPE_NONE);
worker_unlock();
- return has_job;
+ return rc;
}

/*
diff --git a/worker.h b/worker.h
index 56124b8..56737a3 100644
--- a/worker.h
+++ b/worker.h
@@ -1,31 +1,44 @@
#ifndef _WORKER_H
#define _WORKER_H

-#define JOB_TYPE_NONE 0
-#define JOB_TYPE_ANY -1
+/*
+ * These types are used to determine what jobs we should cancel.
+ * For example ":load" cancels jobs for the current view before loading
+ * new playlist.
+ *
+ * The order of the types determines the priority for the job scheduler!
+ */
+enum job_type {
+ JOB_TYPE_QUEUE,
+ JOB_TYPE_PL,
+ JOB_TYPE_LIB,
+ NR_JOB_TYPE
+};

void worker_init(void);
void worker_exit(void);

/*
- * @type: JOB_TYPE_* (>0)
+ * @type: JOB_TYPE_*
* @job_cb: does the job
* @free_cb: frees @data
* @data: data for the callbacks
*/
-void worker_add_job(int type, void (*job_cb)(void *data),
- void (*free_cb)(void *data), void *data);
+void worker_add_job(enum job_type type, void (*job_cb)(void *data),
+ void (*free_cb)(void *data), void *data);

-/*
- * @type: job type. >0, use JOB_TYPE_ANY to remove all
- */
-void worker_remove_jobs(int type);
+void worker_prepend_job(enum job_type type, void (*job_cb)(void *data),
+ void (*free_cb)(void *data), void *data);
+
+void worker_amend_current_job(void *data);

-int worker_has_job(int type);
+void worker_remove_jobs(enum job_type type);
+void worker_remove_all_jobs(void);
+
+int worker_has_job(enum job_type type);
+int worker_has_any_jobs(void);

/*
- * @type: type of this job
- *
* returns: 0 or 1
*
* long jobs should call this to see whether it should cancel

[Gregory Petrosyan]

 This way it is possible to prioritize play_queue and playlist jobs
 over library jobs (because they are smaller and probably more
 important for next song). Also the code gets cleaner!
* When adding directories recursively, each subdirectory becomes a new
 job. This allows insertion of new jobs (see below). It also means
 directories are now processed breadth-first, which might improve
 cache locality.
* A new :add to library prepends a job. The order is not important for
 the library, and this way it is possible to insert jobs before a
 currently running job.

Looks fine, merged to -pu, thanks!

                Gregory

[Gregory Petrosyan]

Post by Johannes Weißl
* A new :add to library prepends a job. The order is not important for
 the library, and this way it is possible to insert jobs before a
 currently running job.

Are you sure this is working? When I "a" a bunch of directories in a
row, they should be added in reverse order, right? For me, the are
added in normal order.

As a side note, visually adding a bunch of directories is much slower
now (they appear one-by-one, instead of all at once).

                Gregory

[Johannes Weißl]

Post by Gregory Petrosyan

Post by Johannes Weißl
* A new :add to library prepends a job. The order is not important for
 the library, and this way it is possible to insert jobs before a
 currently running job.

Are you sure this is working? When I "a" a bunch of directories in a
row, they should be added in reverse order, right? For me, the are
added in normal order.

Hmm, it works only for "flat" directories (containing only files).

Post by Gregory Petrosyan
As a side note, visually adding a bunch of directories is much slower
now (they appear one-by-one, instead of all at once).

I think you can remove the patch again from pu, it is definitely not as
good as I thought. Maybe after some thought something for 2.5...


Johannes

[Johannes Weißl]

if multiple filename or lib_filename lines are in resume file
---
options.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/options.c b/options.c
index 80b13bf..adebc4f 100644
--- a/options.c
+++ b/options.c
@@ -1148,10 +1148,12 @@ static int handle_resume_line(void *data, const char *line)
if (strcmp(cmd, "status") == 0) {
parse_enum(arg, 0, NR_PLAYER_STATUS, player_status_names, (int *) &resume->status);
} else if (strcmp(cmd, "file") == 0) {
+ free(resume->filename);
resume->filename = xstrdup(unescape(arg));
} else if (strcmp(cmd, "position") == 0) {
str_to_int(arg, &resume->position);
} else if (strcmp(cmd, "lib_file") == 0) {
+ free(resume->lib_filename);
resume->lib_filename = xstrdup(unescape(arg));
} else if (strcmp(cmd, "view") == 0) {
parse_enum(arg, 0, NR_VIEWS, view_names, &resume->view);

[Johannes Weißl]

---
ui_curses.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/ui_curses.c b/ui_curses.c
index f0efd9f..832c92f 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -98,6 +98,7 @@ int using_utf8 = 0;

static char *lib_autosave_filename;
static char *pl_autosave_filename;
+static char *play_queue_autosave_filename;

/* shown error message and time stamp
* error is cleared if it is older than 3s and key was pressed
@@ -2210,9 +2211,12 @@ static void init_all(void)

lib_autosave_filename = xstrjoin(cmus_config_dir, "/lib.pl");
pl_autosave_filename = xstrjoin(cmus_config_dir, "/playlist.pl");
+ play_queue_autosave_filename = xstrjoin(cmus_config_dir, "/queue.pl");
pl_filename = xstrdup(pl_autosave_filename);
lib_filename = xstrdup(lib_autosave_filename);

+ if (resume_cmus)
+ cmus_add(play_queue_append, play_queue_autosave_filename, FILE_TYPE_PL, JOB_TYPE_QUEUE);
cmus_add(lib_add_track, lib_autosave_filename, FILE_TYPE_PL, JOB_TYPE_LIB);
cmus_add(pl_add_track, pl_autosave_filename, FILE_TYPE_PL, JOB_TYPE_PL);

@@ -2240,6 +2244,8 @@ static void exit_all(void)

server_exit();
cmus_exit();
+ if (resume_cmus)
+ cmus_save(play_queue_for_each, play_queue_autosave_filename);
cmus_save(lib_for_each, lib_autosave_filename);
cmus_save(pl_for_each, pl_autosave_filename);

[Jason Woofenden]

Wow, that's a small patch! We must have a nice code-base. Perhaps
one of these days I'll dive in again.

I just updated to your latest "next" branch, and it's working
great. Both the live-filter restore and the queue restore.

Thanks again! - Jason

Post by Johannes Weißl
---
ui_curses.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/ui_curses.c b/ui_curses.c
index f0efd9f..832c92f 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -98,6 +98,7 @@ int using_utf8 = 0;
static char *lib_autosave_filename;
static char *pl_autosave_filename;
+static char *play_queue_autosave_filename;
/* shown error message and time stamp
* error is cleared if it is older than 3s and key was pressed
@@ -2210,9 +2211,12 @@ static void init_all(void)
lib_autosave_filename = xstrjoin(cmus_config_dir, "/lib.pl");
pl_autosave_filename = xstrjoin(cmus_config_dir, "/playlist.pl");
+ play_queue_autosave_filename = xstrjoin(cmus_config_dir, "/queue.pl");
pl_filename = xstrdup(pl_autosave_filename);
lib_filename = xstrdup(lib_autosave_filename);
+ if (resume_cmus)
+ cmus_add(play_queue_append, play_queue_autosave_filename, FILE_TYPE_PL, JOB_TYPE_QUEUE);
cmus_add(lib_add_track, lib_autosave_filename, FILE_TYPE_PL, JOB_TYPE_LIB);
cmus_add(pl_add_track, pl_autosave_filename, FILE_TYPE_PL, JOB_TYPE_PL);
@@ -2240,6 +2244,8 @@ static void exit_all(void)
server_exit();
cmus_exit();
+ if (resume_cmus)
+ cmus_save(play_queue_for_each, play_queue_autosave_filename);
cmus_save(lib_for_each, lib_autosave_filename);
cmus_save(pl_for_each, pl_autosave_filename);
--
1.7.4.1
------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev

[Johannes Weißl]

Hi Jason,

Post by Jason Woofenden
Wow, that's a small patch! We must have a nice code-base.

Yes, the code base is generally very nice, if you can find the right
functions :-)! The resume-patch was a lot bigger in it's first version,
but then I found a lot of handy helper functions like parse_enum()!

Post by Jason Woofenden
Perhaps one of these days I'll dive in again.

That would be great, I'm sure there are lots of things to be
fixed/added!


Johannes

[Gregory Petrosyan]

Excellent stuff, as always!

I've merged it to jw/resume and pu -- in a couple of days will merge to master.

Thanks a lot!

Gregory

[Gregory Petrosyan]

On Wed, Apr 6, 2011 at 1:09 AM, Gregory Petrosyan

Post by Gregory Petrosyan
Excellent stuff, as always!
I've merged it to jw/resume and pu -- in a couple of days will merge to master.
Thanks a lot!

FYI: -pu cmus hangs on startup for me now (on OS X, no ~/.cmus/rc
file). Master works fine.

[MacBook-Pro-Gregory-Petrosyan cmus (pu)]$ cat ~/cmus-debug.txt
main: charset = 'UTF-8'
player_init: using real-time scheduling
player_init: setting priority to 47

Don't have time to investigate right now, sorry.

                Gregory

[Gregory Petrosyan]

On Wed, Apr 6, 2011 at 1:32 AM, Gregory Petrosyan

Post by Gregory Petrosyan
On Wed, Apr 6, 2011 at 1:09 AM, Gregory Petrosyan

Post by Gregory Petrosyan
Excellent stuff, as always!
I've merged it to jw/resume and pu -- in a couple of days will merge to master.
Thanks a lot!

FYI: -pu cmus hangs on startup for me now (on OS X, no ~/.cmus/rc
file). Master works fine.

That was my fault — development patch accidentally sneaked into the
branch. Fixed now!

                Gregory

[Johannes Weißl]

---
lib.c | 23 +++++++++++++++++------
lib.h | 2 ++
options.c | 30 ++++++++++++++++++++++++------
3 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/lib.c b/lib.c
index 28ae6d8..b29d6f9 100644
--- a/lib.c
+++ b/lib.c
@@ -428,6 +428,21 @@ struct tree_track *lib_find_track(struct track_info *ti)
return NULL;
}

+void lib_store_cur_track(struct track_info *ti)
+{
+ if (cur_track_ti)
+ track_info_unref(cur_track_ti);
+ cur_track_ti = ti;
+ track_info_ref(cur_track_ti);
+}
+
+struct track_info *lib_get_cur_stored_track(void)
+{
+ if (cur_track_ti)
+ return cur_track_ti;
+ return NULL;
+}
+
static void restore_cur_track(struct track_info *ti)
{
struct tree_track *tt = lib_find_track(ti);
@@ -443,12 +458,8 @@ static int is_filtered_cb(void *data, struct track_info *ti)
static void do_lib_filter(int clear_before)
{
/* try to save cur_track */
- if (lib_cur_track) {
- if (cur_track_ti)
- track_info_unref(cur_track_ti);
- cur_track_ti = tree_track_info(lib_cur_track);
- track_info_ref(cur_track_ti);
- }
+ if (lib_cur_track)
+ lib_store_cur_track(tree_track_info(lib_cur_track));

if (clear_before)
d_print("filter results could grow, clear tracks and re-add (slow)\n");
diff --git a/lib.h b/lib.h
index 3d6fe16..4be3720 100644
--- a/lib.h
+++ b/lib.h
@@ -106,6 +106,8 @@ int lib_for_each_filtered(int (*cb)(void *data, struct track_info *ti), void *da

struct tree_track *lib_find_track(struct track_info *ti);
struct track_info *lib_set_track(struct tree_track *track);
+void lib_store_cur_track(struct track_info *ti);
+struct track_info *lib_get_cur_stored_track(void);

struct tree_track *tree_get_selected(void);
struct track_info *tree_set_selected(void);
diff --git a/options.c b/options.c
index adebc4f..08905a9 100644
--- a/options.c
+++ b/options.c
@@ -1133,6 +1133,7 @@ struct resume {
long int position;
char *lib_filename;
int view;
+ char *live_filter;
};

static int handle_resume_line(void *data, const char *line)
@@ -1157,6 +1158,9 @@ static int handle_resume_line(void *data, const char *line)
resume->lib_filename = xstrdup(unescape(arg));
} else if (strcmp(cmd, "view") == 0) {
parse_enum(arg, 0, NR_VIEWS, view_names, &resume->view);
+ } else if (strcmp(cmd, "live-filter") == 0) {
+ free(resume->live_filter);
+ resume->live_filter = xstrdup(unescape(arg));
}

free(arg);
@@ -1187,11 +1191,15 @@ void resume_load(void)
editable_lock();
lib_add_track(ti);
track_info_unref(ti);
- ti = lib_set_track(lib_find_track(ti));
- BUG_ON(ti != old);
+ lib_store_cur_track(ti);
track_info_unref(ti);
- tree_sel_current();
- sorted_sel_current();
+ ti = lib_set_track(lib_find_track(ti));
+ if (ti) {
+ BUG_ON(ti != old);
+ track_info_unref(ti);
+ tree_sel_current();
+ sorted_sel_current();
+ }
editable_unlock();
}
free(resume.lib_filename);
@@ -1207,6 +1215,12 @@ void resume_load(void)
}
free(resume.filename);
}
+ if (resume.live_filter) {
+ editable_lock();
+ filters_set_live(resume.live_filter);
+ editable_unlock();
+ free(resume.live_filter);
+ }
}

void resume_exit(void)
@@ -1230,11 +1244,15 @@ void resume_exit(void)
fprintf(f, "position %d\n", player_info.pos);
}
player_info_unlock();
- if (lib_cur_track) {
+ if (lib_cur_track)
ti = tree_track_info(lib_cur_track);
+ else
+ ti = lib_get_cur_stored_track();
+ if (ti)
fprintf(f, "lib_file %s\n", escape(ti->filename));
- }
fprintf(f, "view %s\n", view_names[cur_view]);
+ if (lib_live_filter)
+ fprintf(f, "live-filter %s\n", escape(lib_live_filter));

fclose(f);
}

[Johannes Weißl]

Otherwise this happens:
1. start playing a track from the library
2. clear library view
3. quit and start cmus again
4. track is in library view (which shouldn't be!)
---
lib.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib.c b/lib.c
index 37e1eed..5082760 100644
--- a/lib.c
+++ b/lib.c
@@ -438,7 +438,7 @@ void lib_store_cur_track(struct track_info *ti)

struct track_info *lib_get_cur_stored_track(void)
{
- if (cur_track_ti)
+ if (cur_track_ti && lib_find_track(cur_track_ti))
return cur_track_ti;
return NULL;
}