mpv -vo sixel is far superior to ncplayer -bpixel

[dankamongmen]

@dnkl pointed this out in #1380, but it's worth its own issue. Get two xterms up, along with a recent mpv build featuring libsixel support. Run it alongside ncplayer -bpixel. mpv's superiority is obvious (we beat it with cell output, though). Get to this level of output, in both speed and quality.

[dnkl]

https://github.com/dankamongmen/notcurses/commit/68c60fe533615574040a21c82237654a6c96e19f gives me:

(gdb) bt
#0  0x00007ffff7b83971 in __memset_avx2_erms () from /usr/lib/libc.so.6
#1  0x00007ffff7f9d002 in sixel_blit (nc=0x5555555be3d0, linesize=3776, data=0x5555559ccf00, begy=0, begx=0, leny=531, lenx=944, 
    bargs=0x7fffffffdb20) at ../../src/lib/sixel.c:418
#2  0x00007ffff7fb7f0a in rgba_blit_dispatch (bargs=0x7fffffffdb20, lenx=944, leny=531, begx=0, begy=0, data=<optimized out>, 
    linesize=<optimized out>, bset=0x7ffff7fb39d0 <notcurses_blitters+336>, nc=0x5555555be3d0) at ../../src/lib/internal.h:1272
#3  ffmpeg_blit (ncv=ncv@entry=0x5555555c17e0, rows=531, cols=944, n=n@entry=0x5555555be3d0, 
    bset=bset@entry=0x7ffff7fb39d0 <notcurses_blitters+336>, begy=begy@entry=0, begx=<optimized out>, leny=<optimized out>, 
    lenx=<optimized out>, bargs=<optimized out>) at ../../src/media/ffmpeg.cpp:525
#4  0x00007ffff7f9f9b2 in ncvisual_blit (ncv=ncv@entry=0x5555555c17e0, rows=<optimized out>, cols=<optimized out>, 
    n=n@entry=0x5555555be3d0, bset=bset@entry=0x7ffff7fb39d0 <notcurses_blitters+336>, begy=begy@entry=0, begx=0, leny=531, lenx=944, 
    barg=0x7fffffffdb20) at ../../src/lib/visual.cpp:21
#5  0x00007ffff7fa05ef in ncvisual_render_pixels (tcache=tcache@entry=0x5555555a90e0, ncv=ncv@entry=0x5555555c17e0, 
    bset=bset@entry=0x7ffff7fb39d0 <notcurses_blitters+336>, placey=placey@entry=1, placex=placex@entry=0, begy=begy@entry=0, begx=0, 
    n=0x5555555be3d0, scaling=NCSCALE_SCALE, stdn=0x5555555be3d0) at ../../src/lib/visual.cpp:533
#6  0x00007ffff7fa0b48 in ncvisual_render (nc=nc@entry=0x5555555a6e50, ncv=ncv@entry=0x5555555c17e0, vopts=vopts@entry=0x7fffffffdc80)
    at ../../src/lib/visual.cpp:589
#7  0x00007ffff7fb79c3 in ffmpeg_stream (nc=0x5555555a6e50, ncv=0x5555555c17e0, timescale=1, 
    streamer=0x555555557a0d <perframe(ncvisual*, ncvisual_options*, timespec const*, void*)>, vopts=0x7fffffffddc0, 
    curry=0x7fffffffdd70) at ../../src/media/ffmpeg.cpp:410
#8  0x00007ffff7fa1082 in ncvisual_stream (nc=<optimized out>, ncv=ncv@entry=0x5555555c17e0, timescale=timescale@entry=1, 
    streamer=streamer@entry=0x555555557a0d <perframe(ncvisual*, ncvisual_options*, timespec const*, void*)>, 
    vopts=vopts@entry=0x7fffffffddc0, curry=curry@entry=0x7fffffffdd70) at ../../src/lib/visual.cpp:735
#9  0x0000555555559730 in ncpp::Visual::stream (curry=0x7fffffffdd70, 
    streamer=0x555555557a0d <perframe(ncvisual*, ncvisual_options*, timespec const*, void*)>, timescale=1, vopts=0x7fffffffddc0, 
    this=<optimized out>) at ../../include/ncpp/Visual.hh:76
#10 main (argc=6, argv=0x7fffffffdfa8) at ../../src/player/play.cpp:388

edit: command was ncplayer -b pixel -s scale <video.webm>

[dnkl]

@dankamongmen should I bisect?

[dnkl]

#1  0x00007ffff7f9d002 in sixel_blit (nc=0x5555555be3d0, linesize=3776, data=0x5555559ccf00, begy=0, begx=0, leny=531, lenx=944, 
    bargs=0x7fffffffdb20) at ../../src/lib/sixel.c:418
418       memset(stable.data, 0, sixelcount * bargs->pixel.colorregs);
(gdb) p sixelcount
$1 = 84016
(gdb) p bargs->pixel.colorregs
$2 = 1024
(gdb) p colorregs
$3 = 256
(gdb) p stable
$4 = {colors = 0, deets = 0x555555bb6770, data = 0x7fffee907010 "", table = 0x555555634be0 "\260\016\276\367\377\177", 
  sixelcount = 84016, colorregs = 256}

I.e. stable.data is 256*84016, but the memset() sets 1024*84016 bytes.

[dnkl]

But it works in XTerm... hmm

[dnkl]

But it works in XTerm... hmm

Ah, if I bump xterm*numColorRegisters from 256 to 1024, it crashes in XTerm too.

[dnkl]

Given

  int colorregs = bargs->pixel.colorregs;
  if(colorregs > 256){
    colorregs = 256;
  }

This seems like the appropriate solution (and works):

diff --git a/src/lib/sixel.c b/src/lib/sixel.c
index 46169e7f8..9d117c593 100644
--- a/src/lib/sixel.c
+++ b/src/lib/sixel.c
@@ -415,8 +415,8 @@ int sixel_blit(ncplane* nc, int linesize, const void* data, int begy, int begx,
     return -1;
   }
   // stable.table doesn't need initializing; we start from the bottom
-  memset(stable.data, 0, sixelcount * bargs->pixel.colorregs);
-  memset(stable.deets, 0, sizeof(*stable.deets) * bargs->pixel.colorregs);
+  memset(stable.data, 0, sixelcount * colorregs);
+  memset(stable.deets, 0, sizeof(*stable.deets) * colorregs);
   if(extract_color_table(data, linesize, begy, begx, leny, lenx, &stable)){
     free(stable.table);
     free(stable.data);

[dankamongmen]

i wouldn't mess around with development branch stuff -- it's not generally ready for production -- but i'll certainly take quality investigations and bugfixes where i can find them =]. thanks, friend!

[dankamongmen]

i can similarly raise a coredump using -snone -bpixel ../data/penrose-tiling.png, in both kitty and xterm.

==3791751== Invalid write of size 4
==3791751==    at 0x4886D7C: plane_blit_sixel (internal.h:1104)
==3791751==    by 0x4886D7C: sixel_blit_inner (sixel.c:261)
==3791751==    by 0x4887341: sixel_blit (sixel.c:294)
==3791751==    by 0x4851EB3: rgba_blit_dispatch (internal.h:1261)
==3791751==    by 0x4851EB3: ffmpeg_blit(ncvisual*, int, int, ncplane*, blitset const*, int, int, int, int, blitterargs const*) (ffmpeg.cpp:525)
==3791751==    by 0x488A4D4: ncvisual_blit (visual.cpp:21)
==3791751==    by 0x488B0D5: ncvisual_render_pixels(tinfo*, ncvisual*, blitset const*, int, int, int, int, ncplane*, ncscale_e, ncplane*) (visual.cpp:533)
==3791751==    by 0x488B550: ncvisual_render (visual.cpp:589)
==3791751==    by 0x4851AD1: ffmpeg_stream(notcurses*, ncvisual*, float, int (*)(ncvisual*, ncvisual_options*, timespec const*, void*), ncvisual_options const*, void*) (ffmpeg.cpp:410)
==3791751==    by 0x10BBED: stream (Visual.hh:76)
==3791751==    by 0x10BBED: main (play.cpp:388)
==3791751==  Address 0xeb30e80 is 0 bytes after a block of size 78,080 alloc'd
==3791751==    at 0x483AB65: calloc (vg_replace_malloc.c:760)
==3791751==    by 0x4874CEB: ncplane_new_internal (notcurses.c:312)
==3791751==    by 0x4876803: create_initial_ncplane (notcurses.c:402)
==3791751==    by 0x4876803: notcurses_core_init (notcurses.c:1033)
==3791751==    by 0x484A76E: ncpp::NotCurses::NotCurses(notcurses_options const&, _IO_FILE*) (NotCurses.cc:37)
==3791751==    by 0x10BA50: main (play.cpp:349)
==3791751== 
==3791751== Invalid write of size 1
==3791751==    at 0x4886D7F: plane_blit_sixel (internal.h:1105)
==3791751==    by 0x4886D7F: sixel_blit_inner (sixel.c:261)
==3791751==    by 0x4887341: sixel_blit (sixel.c:294)
==3791751==    by 0x4851EB3: rgba_blit_dispatch (internal.h:1261)
==3791751==    by 0x4851EB3: ffmpeg_blit(ncvisual*, int, int, ncplane*, blitset const*, int, int, int, int, blitterargs const*) (ffmpeg.cpp:525)
==3791751==    by 0x488A4D4: ncvisual_blit (visual.cpp:21)
==3791751==    by 0x488B0D5: ncvisual_render_pixels(tinfo*, ncvisual*, blitset const*, int, int, int, int, ncplane*, ncscale_e, ncplane*) (visual.cpp:533)
==3791751==    by 0x488B550: ncvisual_render (visual.cpp:589)
==3791751==    by 0x4851AD1: ffmpeg_stream(notcurses*, ncvisual*, float, int (*)(ncvisual*, ncvisual_options*, timespec const*, void*), ncvisual_options const*, void*) (ffmpeg.cpp:410)
==3791751==    by 0x10BBED: stream (Visual.hh:76)
==3791751==    by 0x10BBED: main (play.cpp:388)
==3791751==  Address 0xeb30e85 is 5 bytes after a block of size 78,080 alloc'd
==3791751==    at 0x483AB65: calloc (vg_replace_malloc.c:760)
==3791751==    by 0x4874CEB: ncplane_new_internal (notcurses.c:312)
==3791751==    by 0x4876803: create_initial_ncplane (notcurses.c:402)
==3791751==    by 0x4876803: notcurses_core_init (notcurses.c:1033)
==3791751==    by 0x484A76E: ncpp::NotCurses::NotCurses(notcurses_options const&, _IO_FILE*) (NotCurses.cc:37)
==3791751==    by 0x10BA50: main (play.cpp:349)

[dankamongmen]

this might just be from a huge allocation. -snone on this 4Kx4K image yields 2.8 meagsixels. by 256 colors gets...nah, just 715MB. that oughtn't be a problem (not a functional one on my workstation anyway).

[dankamongmen]

we blow up at y: 178 x: 372

[dankamongmen]

y: 178 x: 372 leny: 179 lenx: 373 innnnnnnnnteresting

[dankamongmen]

y: 178 x: 372 leny: 179 lenx: 373 dimy: 61 dimx: 80 ahhhh there's our problem

[dankamongmen]

the coredump is remedied