visual_implementation: use weak symbol rather than library constructors #1301

src/lib/direct.cpp

@@ -669,7 +669,7 @@ ncdirect* ncdirect_init(const char* termtype, FILE* outfp, uint64_t flags){
     goto err;
   }
   shortname_term = termname();
-  if(ncvisual_init(ffmpeg_log_level(NCLOGLEVEL_SILENT))){
+  if(ncvisual_init(NCLOGLEVEL_SILENT)){
     goto err;
   }
   if(interrogate_terminfo(&ret->tcache, shortname_term)){

src/lib/internal.h

@@ -8,19 +8,6 @@ extern "C" {
 #include "version.h"
 #include "builddef.h"
 
-#ifdef USE_FFMPEG
-#include <libavutil/error.h>
-#include <libavutil/frame.h>
-#include <libavutil/pixdesc.h>
-#include <libavutil/version.h>
-#include <libswscale/version.h>
-#include <libavformat/version.h>
-#else
-#ifdef USE_OIIO
-const char* oiio_version(void);
-#endif
-#endif
-
 #include <term.h>
 #include <time.h>
 #include <stdio.h>
@@ -42,6 +29,7 @@ const char* oiio_version(void);
 #include "egcpool.h"
 
 struct esctrie;
+struct ncvisual_details;
 
 // we can't define multipart ncvisual here, because OIIO requires C++ syntax,
 // and we can't go throwing C++ syntax into this header. so it goes.
@@ -864,9 +852,6 @@ int get_controlling_tty(void);
     nclog("%s:%d:" fmt, __func__, __LINE__, ##__VA_ARGS__); } \
   } }while(0);
 
-// Convert a notcurses log level to some multimedia library equivalent.
-int ffmpeg_log_level(ncloglevel_e level);
-
 int term_setstyle(FILE* out, unsigned cur, unsigned targ, unsigned stylebit,
                   const char* ton, const char* toff);
 
@@ -1074,6 +1059,29 @@ int drop_signals(void* nc);
 
 void ncvisual_printbanner(const notcurses* nc);
 
+typedef struct ncvisual_implementation {
+  int (*ncvisual_init)(int loglevel);
+  int (*ncvisual_decode)(struct ncvisual*);
+  int (*ncvisual_blit)(struct ncvisual* ncv, int rows, int cols, ncplane* n,
+                       const struct blitset* bset, int placey, int placex,
+                       int begy, int begx, int leny, int lenx,
+                       bool blendcolors);
+  struct ncvisual* (*ncvisual_create)(void);
+  struct ncvisual* (*ncvisual_from_file)(const char* s);
+  void (*ncvisual_printbanner)(const struct notcurses* nc);
+  // ncv constructors other than ncvisual_from_file() need to set up the
+  // AVFrame* 'frame' according to their own data, which is assumed to
+  // have been prepared already in 'ncv'.
+  void (*ncvisual_details_seed)(struct ncvisual* ncv);
+  void (*ncvisual_details_destroy)(struct ncvisual_details* deets);
+  bool canopen_images;
+  bool canopen_videos;
+} ncvisual_implementation;
+
+// overriden by strong symbol in libnotcurses.so if built with multimedia
+extern const ncvisual_implementation* visual_implementation
+ __attribute__ ((visibility("default")));
+
 #ifdef __cplusplus
 }
 #endif

src/lib/notcurses.c

@@ -754,28 +754,6 @@ void notcurses_stats_reset(notcurses* nc, ncstats* stats){
   stash_stats(nc);
 }
 
-// Convert a notcurses log level to some multimedia library equivalent.
-int ffmpeg_log_level(ncloglevel_e level){
-#ifdef USE_FFMPEG
-  switch(level){
-    case NCLOGLEVEL_SILENT: return AV_LOG_QUIET;
-    case NCLOGLEVEL_PANIC: return AV_LOG_PANIC;
-    case NCLOGLEVEL_FATAL: return AV_LOG_FATAL;
-    case NCLOGLEVEL_ERROR: return AV_LOG_ERROR;
-    case NCLOGLEVEL_WARNING: return AV_LOG_WARNING;
-    case NCLOGLEVEL_INFO: return AV_LOG_INFO;
-    case NCLOGLEVEL_VERBOSE: return AV_LOG_VERBOSE;
-    case NCLOGLEVEL_DEBUG: return AV_LOG_DEBUG;
-    case NCLOGLEVEL_TRACE: return AV_LOG_TRACE;
-    default: break;
-  }
-  fprintf(stderr, "Invalid log level: %d\n", level);
-  return AV_LOG_TRACE;
-#else
-  return level;
-#endif
-}
-
 // unless the suppress_banner flag was set, print some version information and
 // (if applicable) warnings to stdout. we are not yet on the alternate screen.
 static void
@@ -1045,7 +1023,7 @@ notcurses* notcurses_init(const notcurses_options* opts, FILE* outfp){
     terminfostr(&ret->tcache.smcup, "smcup");
     terminfostr(&ret->tcache.rmcup, "rmcup");
   }
-  if(ncvisual_init(ffmpeg_log_level(ret->loglevel))){
+  if(ncvisual_init(ret->loglevel)){
     goto err;
   }
   ret->stdplane = NULL;

src/lib/visual-details.h

@@ -16,29 +16,6 @@ typedef struct ncvisual {
   bool owndata; // we own data iff owndata == true
 } ncvisual;
 
-typedef struct ncvisual_implementation {
-  int (*ncvisual_init)(int loglevel);
-  int (*ncvisual_decode)(ncvisual*);
-  int (*ncvisual_blit)(ncvisual* ncv, int rows, int cols, ncplane* n,
-                       const struct blitset* bset, int placey, int placex,
-                       int begy, int begx, int leny, int lenx,
-                       bool blendcolors);
-  ncvisual* (*ncvisual_create)(void);
-  ncvisual* (*ncvisual_from_file)(const char* s);
-  void (*ncvisual_printbanner)(const struct notcurses* nc);
-  // ncv constructors other than ncvisual_from_file() need to set up the
-  // AVFrame* 'frame' according to their own data, which is assumed to
-  // have been prepared already in 'ncv'.
-  void (*ncvisual_details_seed)(ncvisual* ncv);
-  void (*ncvisual_details_destroy)(struct ncvisual_details* deets);
-  bool canopen_images;
-  bool canopen_videos;
-} ncvisual_implementation;
-
-// ugh! need export this for pluggable multimedia modules without dlopen()
-__attribute__((visibility("default")))
-int notcurses_set_ncvisual_implementation(const ncvisual_implementation* imp);
-
 static inline auto
 ncvisual_set_data(ncvisual* ncv, uint32_t* data, bool owned) -> void {
   if(ncv->owndata){

src/lib/visual.cpp

@@ -4,31 +4,13 @@
 #include "visual-details.h"
 #include "internal.h"
 
-// FIXME make this a weak symbol instead so we work with static linking
-static const ncvisual_implementation* impl;
-static pthread_rwlock_t impllock = PTHREAD_RWLOCK_INITIALIZER;
-
-int notcurses_set_ncvisual_implementation(const ncvisual_implementation* imp){
-  int ret = -1;
-  if(pthread_rwlock_wrlock(&impllock)){
-    return -1;
-  }
-  if(impl == nullptr){
-    impl = imp;
-  }
-  ret |= pthread_rwlock_unlock(&impllock);
-  return ret;
-}
+const __attribute__ ((weak)) ncvisual_implementation* visual_implementation = nullptr;
 
 auto ncvisual_decode(ncvisual* nc) -> int {
   int ret = -1;
-  if(pthread_rwlock_rdlock(&impllock)){
-    return -1;
-  }
-  if(impl){
-    ret = impl->ncvisual_decode(nc);
+  if(visual_implementation){
+    ret = visual_implementation->ncvisual_decode(nc);
   }
-  ret |= pthread_rwlock_unlock(&impllock);
   return ret;
 }
 
@@ -37,11 +19,8 @@ auto ncvisual_blit(ncvisual* ncv, int rows, int cols, ncplane* n,
                    int begy, int begx, int leny, int lenx,
                    bool blendcolors) -> int {
   int ret = -1;
-  if(pthread_rwlock_rdlock(&impllock)){
-    return -1;
-  }
-  if(impl){
-    if(impl->ncvisual_blit(ncv, rows, cols, n, bset, placey, placex,
+  if(visual_implementation){
+    if(visual_implementation->ncvisual_blit(ncv, rows, cols, n, bset, placey, placex,
                            begy, begx, leny, lenx, blendcolors) >= 0){
       ret = 0;
     }
@@ -51,63 +30,48 @@ auto ncvisual_blit(ncvisual* ncv, int rows, int cols, ncplane* n,
       ret = 0;
     }
   }
-  ret |= pthread_rwlock_unlock(&impllock);
   return ret;
 }
 
 auto ncvisual_details_seed(struct ncvisual* ncv) -> void {
-  pthread_rwlock_rdlock(&impllock);
-  if(impl){
-    impl->ncvisual_details_seed(ncv);
+  if(visual_implementation){
+    visual_implementation->ncvisual_details_seed(ncv);
   }
-  pthread_rwlock_unlock(&impllock);
 }
 
 auto ncvisual_init(int loglevel) -> int {
   int ret = 0; // default to success here
-  if(pthread_rwlock_rdlock(&impllock)){
-    return -1;
-  }
-  if(impl){
-    ret = impl->ncvisual_init(loglevel);
+  if(visual_implementation){
+    ret = visual_implementation->ncvisual_init(loglevel);
   }
-  ret |= pthread_rwlock_unlock(&impllock);
   return ret;
 }
 
 auto ncvisual_from_file(const char* filename) -> ncvisual* {
   ncvisual* ret = nullptr;
-  if(pthread_rwlock_rdlock(&impllock) == 0){
-    if(impl){
-      ret = impl->ncvisual_from_file(filename);
-    }
-    pthread_rwlock_unlock(&impllock);
+  if(visual_implementation){
+    ret = visual_implementation->ncvisual_from_file(filename);
   }
   return ret;
 }
 
 auto ncvisual_create(void) -> ncvisual* {
   ncvisual* ret = nullptr;
-  if(pthread_rwlock_rdlock(&impllock) == 0){
-    if(impl){
-      ret = impl->ncvisual_create();
-    }else{
-      ret = new ncvisual{};
-    }
-    pthread_rwlock_unlock(&impllock);
+  if(visual_implementation){
+    ret = visual_implementation->ncvisual_create();
+  }else{
+    ret = new ncvisual{};
   }
   return ret;
 }
 
 auto ncvisual_printbanner(const notcurses* nc) -> void {
-  pthread_rwlock_rdlock(&impllock);
-  if(impl){
-    impl->ncvisual_printbanner(nc);
+  if(visual_implementation){
+    visual_implementation->ncvisual_printbanner(nc);
   }else{
     term_fg_palindex(nc, stderr, nc->tcache.colors <= 88 ? 1 % nc->tcache.colors : 0xcb);
     fprintf(stderr, "\n Warning! Notcurses was built without multimedia support.\n");
   }
-  pthread_rwlock_unlock(&impllock);
 }
 
 auto ncvisual_geom(const notcurses* nc, const ncvisual* n,
@@ -585,15 +549,13 @@ auto ncvisual_from_plane(const ncplane* n, ncblitter_e blit, int begy, int begx,
 
 auto ncvisual_destroy(ncvisual* ncv) -> void {
   if(ncv){
-    pthread_rwlock_rdlock(&impllock);
-    if(impl){
-      impl->ncvisual_details_destroy(ncv->details);
+    if(visual_implementation){
+      visual_implementation->ncvisual_details_destroy(ncv->details);
     }
     if(ncv->owndata){
       free(ncv->data);
     }
     delete ncv;
-    pthread_rwlock_unlock(&impllock);
   }
 }
 
@@ -673,17 +635,17 @@ auto ncvisual_polyfill_yx(ncvisual* n, int y, int x, uint32_t rgba) -> int {
 }
 
 auto notcurses_canopen_images(const notcurses* nc __attribute__ ((unused))) -> bool {
-  if(!impl){
+  if(!visual_implementation){
     return false;
   }
-  return impl->canopen_images;
+  return visual_implementation->canopen_images;
 }
 
 auto notcurses_canopen_videos(const notcurses* nc __attribute__ ((unused))) -> bool {
-  if(!impl){
+  if(!visual_implementation){
     return false;
   }
-  return impl->canopen_videos;
+  return visual_implementation->canopen_videos;
 }
 
 auto ncvisual_decode_loop(ncvisual* nc) -> int {

src/media/ffmpeg.cpp

@@ -41,8 +41,6 @@ typedef struct ncvisual_details {
 
 #define IMGALLOCALIGN 32
 
-void inject_implementation(void) __attribute__ ((constructor));
-
 /*static void
 print_frame_summary(const AVCodecContext* cctx, const AVFrame* f) {
   char pfmt[128];
@@ -544,8 +542,29 @@ auto ffmpeg_details_seed(ncvisual* ncv) -> void {
   ncv->details->frame->format = AV_PIX_FMT_RGBA;
 }
 
+auto ffmpeg_log_level(int level) -> int{
+#ifdef USE_FFMPEG
+  switch(level){
+    case NCLOGLEVEL_SILENT: return AV_LOG_QUIET;
+    case NCLOGLEVEL_PANIC: return AV_LOG_PANIC;
+    case NCLOGLEVEL_FATAL: return AV_LOG_FATAL;
+    case NCLOGLEVEL_ERROR: return AV_LOG_ERROR;
+    case NCLOGLEVEL_WARNING: return AV_LOG_WARNING;
+    case NCLOGLEVEL_INFO: return AV_LOG_INFO;
+    case NCLOGLEVEL_VERBOSE: return AV_LOG_VERBOSE;
+    case NCLOGLEVEL_DEBUG: return AV_LOG_DEBUG;
+    case NCLOGLEVEL_TRACE: return AV_LOG_TRACE;
+    default: break;
+  }
+  fprintf(stderr, "Invalid log level: %d\n", level);
+  return AV_LOG_TRACE;
+#else
+  return level;
+#endif
+}
+
 auto ffmpeg_init(int loglevel) -> int {
-  av_log_set_level(loglevel);
+  av_log_set_level(ffmpeg_log_level(loglevel));
   // FIXME could also use av_log_set_callback() and capture the message...
   return 0;
 }
@@ -583,8 +602,6 @@ const static ncvisual_implementation ffmpeg_impl = {
   .canopen_videos = true,
 };
 
-void inject_implementation(void){
-  notcurses_set_ncvisual_implementation(&ffmpeg_impl);
-}
+const ncvisual_implementation* visual_implementation = &ffmpeg_impl;
 
 #endif

src/media/oiio.cpp

@@ -8,8 +8,6 @@
 #include "internal.h"
 #include "visual-details.h"
 
-static void inject_implementation(void) __attribute__ ((constructor));
-
 typedef struct ncvisual_details {
   std::unique_ptr<OIIO::ImageInput> image;  // must be close()d
   std::unique_ptr<uint32_t[]> frame;
@@ -284,8 +282,6 @@ const static ncvisual_implementation oiio_impl = {
   .canopen_videos = false,
 };
 
-static void inject_implementation(void){
-  notcurses_set_ncvisual_implementation(&oiio_impl);
-}
+const ncvisual_implementation* visual_implementation = &oiio_impl;
 
 #endif