From 421f450526bc3e8f39a61c3c9acb66d7b8fc9940 Mon Sep 17 00:00:00 2001 From: nick black Date: Tue, 19 Jan 2021 04:45:56 -0500 Subject: [PATCH] visual_implementation: use weak symbol rather than library constructors #1301 --- src/lib/direct.cpp | 2 +- src/lib/internal.h | 40 +++++++++++-------- src/lib/notcurses.c | 24 +----------- src/lib/visual-details.h | 23 ----------- src/lib/visual.cpp | 84 +++++++++++----------------------------- src/media/ffmpeg.cpp | 29 +++++++++++--- src/media/oiio.cpp | 6 +-- 7 files changed, 73 insertions(+), 135 deletions(-) diff --git a/src/lib/direct.cpp b/src/lib/direct.cpp index afe9993a9..9bdbc35a4 100644 --- a/src/lib/direct.cpp +++ b/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)){ diff --git a/src/lib/internal.h b/src/lib/internal.h index 48035ea81..aa9d3ccfc 100644 --- a/src/lib/internal.h +++ b/src/lib/internal.h @@ -8,19 +8,6 @@ extern "C" { #include "version.h" #include "builddef.h" -#ifdef USE_FFMPEG -#include -#include -#include -#include -#include -#include -#else -#ifdef USE_OIIO -const char* oiio_version(void); -#endif -#endif - #include #include #include @@ -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 diff --git a/src/lib/notcurses.c b/src/lib/notcurses.c index 2a769596c..312c2dd3f 100644 --- a/src/lib/notcurses.c +++ b/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; diff --git a/src/lib/visual-details.h b/src/lib/visual-details.h index f2d9a2916..99c0fff36 100644 --- a/src/lib/visual-details.h +++ b/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){ diff --git a/src/lib/visual.cpp b/src/lib/visual.cpp index 070880131..005428234 100644 --- a/src/lib/visual.cpp +++ b/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 { diff --git a/src/media/ffmpeg.cpp b/src/media/ffmpeg.cpp index 2e608fe39..cafd7bd27 100644 --- a/src/media/ffmpeg.cpp +++ b/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 diff --git a/src/media/oiio.cpp b/src/media/oiio.cpp index b2f98d7b2..cd7d67035 100644 --- a/src/media/oiio.cpp +++ b/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 image; // must be close()d std::unique_ptr 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