Skip to content
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.

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

Use ncurses instead of printing a lot? #4

Closed
anko opened this issue Apr 24, 2015 · 12 comments · Fixed by #35
Closed

Use ncurses instead of printing a lot? #4

anko opened this issue Apr 24, 2015 · 12 comments · Fixed by #35

Comments

@anko
Copy link
Contributor

anko commented Apr 24, 2015

At the moment, cava renders each frame by printing term_width * term_height characters to fill the terminal.

This could be sped up using ncurses to only erase and re-print what changed.


Example:

Going from bars like

#   #
##  #
### #
#####

to

#    
##  #
#####
#####

would just mean doing this (d for delete, i for insert)

    d

   i 

with two cursor jumps, instead of re-printing the whole lot.

@karlstav
Copy link
Owner

Sounds like a good idea. I wasn't aware that such a thing as ncurses existed when i started working on cava. But reading about it now it sounds like just the thing.

There is however a functionality in cava where it saves the last frame in a matrix and then only prints the changes. This saves a lot of CPU, but it still has to move the cursor along x number of characters where there is no changes. Ncurses might also improve the overall look, as it still is a bit glitchy in larger terminal screens.

I'll note it down in the todo list! Hopefully I will get around to it soon, but it's hard to tell when. Feel free to have a crack at it. All bar heights are stored in the f[] array.

@karlstav
Copy link
Owner

karlstav commented May 7, 2015

I started playing around with ncurses and i think the transition would be verry simple. Since ncurses prints to a buffer instead of printing directly to the stdio, we could just reprint each bar every cycle and then refresh. The problem with printf is that it uses a lot of cpu, hence the complicated system of saving each frame in a matrix and then checking for changes every cycle. So ncurses would also save alot of cpu (cava still uses like 20-30% cpu on my rpi).

But I still haven't been able to print a single wide- (unicode) chrachter. And we are still going to need those in order to print the 1/8- 7/8 block characters right? Or can anyone think of another trick to get the increased resolution?

I read that wide chrachters with ncurses in c programming could be a problem, help?

@livibetter
Copy link
Contributor

@karlstav that website seems to be blocking me for no reason.

Anyway, I have virtually zero knowledge of ncurses, but I do see this a few times.

The normal ncurses libraries support 8-bit characters. The ncurses library can also be configured (--enable-widec) to support wide-characters (for instance Unicode and the UTF-8 encoding). The corresponding wide-character ncursesw libraries are source-compatible with the normal applications. That is, applications must be compiled and linked against the ncursesw library.

@karlstav
Copy link
Owner

I made a separate ncurses branch, it might actually already be done. If I regret one thing it is not using ncurses from the start, it was a lot simpler and looks a bit smoother too. I haven't done much testing @anko do you want to check it out before I merge it with the main branch?

Also I don't know how to install the dev files in arch, @CelestialWalrus do you want to check it out? According to this old thread it might be a bit different from debian.

@ghost
Copy link

ghost commented May 14, 2015

@karlstav Just install the ncurses package. In Arch "dev" files come with every package.

@karlstav
Copy link
Owner

ok, but it seems that it then only has to include <ncurses.h>, this differs from debian/ubuntu where you have to include "ncursesw/curses.h". Do we have to make a "configuration"?

@anko
Copy link
Contributor Author

anko commented May 16, 2015

What we're seeing as subtle differences in dependencies is because of this (from the ncurses FAQ; relevant bits highlighted):

The ncurses and ncursesw libraries are reasonably source-compatible. That is, an application written for "ncurses" will build with "ncursesw". But it will behave differently in response to your locale settings. (Some distributors, who do not care about the differences, have chosen to merge the names together as "ncurses").

Distros' "ncursesw" packages (or just "ncurses" on ones that don't distinguish) should include a program ncursesw5-config that outputs platform-specific appropriate ld flags.

Does it compile on Debian with these changes?

t a/cava.c b/cava.c
index d6b7704..0446ea3 100644
--- a/cava.c
+++ b/cava.c
 -21,7 +21,7 
 #include <time.h>
 #include <getopt.h>
 #include <pthread.h>
-#include "ncursesw/curses.h"
+#include <ncurses.h>
 #include <wchar.h>


diff --git a/makefile b/makefile
index a0d9bd4..97b57bd 100644
--- a/makefile
+++ b/makefile
 -5,7 +5,7  CC       = gcc
 CFLAGS   = -std=c99 -Wall -Wextra
 CPPFLAGS = -DPACKAGE=\"$(PACKAGE)\" -DVERSION=\"$(VERSION)\" \
            -D_POSIX_SOURCE -D _POSIX_C_SOURCE=200809L
-LDLIBS   = -lasound -lm -lfftw3 -lpthread -lncursesw
+LDLIBS   = -lasound -lm -lfftw3 -lpthread $(shell ncursesw5-config --libs)

 INSTALL     = install
 INSTALL_BIN = $(INSTALL) -D -m 755

(OK on Arch and Fedora.)


Functionality-wise, works great! 👍 ✨ 🎈

@karlstav
Copy link
Owner

nope..
without libncurses5-dev installed (only libncursesw5-dev) i get:

gcc -std=c99 -Wall -Wextra -DPACKAGE=\"cava\" -DVERSION=\"0.1.0-11-gad45eea-dirty\" -D_POSIX_SOURCE -D _POSIX_C_SOURCE=200809L   cava.c  -lasound -lm -lfftw3 -lpthread -lncursesw -ltinfo -o cava
cava.c:24:21: fatal error: ncurses.h: No such file or directory
 #include <ncurses.h>
                     ^
compilation terminated.
make: *** [cava] Error 1

with libncursesw5-dev installed i get:

gcc -std=c99 -Wall -Wextra -DPACKAGE=\"cava\" -DVERSION=\"0.1.0-11-gad45eea-dirty\" -D_POSIX_SOURCE -D _POSIX_C_SOURCE=200809L   cava.c  -lasound -lm -lfftw3 -lpthread -lncursesw -ltinfo -o cava
cava.c: In function ‘main’:
cava.c:683:9: warning: implicit declaration of function ‘mvaddwstr’ [-Wimplicit-function-declaration]
         for (n = flastd[i] / 8; n < f[i] / 8; n++) for (q = 0; q < bw; q++) mvaddwstr((height - n), (i * bw) + q + i, bars[7]);

So we get the appropriate ld flags, but I think we need to include "ncursesw/curses.h" when in debian. It works if I include "ncursesw/curses.h" and remove <ncurses.h.>

glad to hear its working, passing 50 ⭐ as well, cause for 🎉 🍸

@anko
Copy link
Contributor Author

anko commented May 16, 2015

I looked further into header file locations. Arch devs’ stance:

Given the vast majority of programs do deal with this properly, I think I will close this as "not a bug".

So Arch needs #include <ncurses.h>. Argh.

Autotools/cmake seem overkill, since it's a one-liner change. I’m unfamiliar with both, but if I had to decide, I’d leave patching it up to packagers (@CelestialWalrus), for now. (Maybe a readme note for Arch users compiling from source?)


Clearing up -dev confusion: You definitely want libncursesw5-dev. Debian applies -dev to library packages containing header files, for compiling other programs against it. libncursesw5 (no -dev suffix) contains only the dynamic library—no headers. Hence gcc complains about missing ncurses.h.

Some Distros handle this differently: Red Hat & Fedora use -devel. Arch Linux packages always include headers.

@karlstav
Copy link
Owner

I really want to make the ncurses branch master. But since a lot of people uses arch, maybe it would be a good idea to have this resolved?

We could #include <ncurses.h> as well, then debian users would also have to install libncurses5-dev as well as libncursesw5-dev. But would arch compile it if #include "ncursesw/curses.h"?

We could always keep the old printf version as a separate branch for compatibility reasons.

anko added a commit to anko/cava that referenced this issue May 20, 2015
@karlstav
Copy link
Owner

Long thread, maybe i should start a new one?

Anyway I need some tips on how to make the merge of the ncurses branch in to the master branch, since it is now also a couple of commits behind.

I was thinking of fist creating a new branch from the master branch called "printf-version" (maybe this could be a separate option for compatibility reasons). Then pulling the changes from the ncurses branch in to the master branch, but how?

@anko
Copy link
Contributor Author

anko commented May 22, 2015

@karlstav

how to make the merge of the ncurses branch into the master branch, since it is now also a couple of commits behind

Merging diverged branches can cause merge conflicts, but they're manually resolveable. That also happened previously with livi's key controls patch. I think I've managed to merge this one well too—I'll PR soon.

I was thinking of first creating a new branch from the master branch called "printf-version" (maybe this could be a separate option for compatibility reasons).

You can create a branch from any commit whenever by doing git checkout <commit id>, then git branch <new branch name>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants