From 4e90dd24e70865e4ffe02706e4b5da4bd2164e9e Mon Sep 17 00:00:00 2001 From: bnnm Date: Sat, 8 Jan 2022 20:57:45 +0100 Subject: [PATCH] Improve STDIO for TXTP that open many small files --- fb2k/foo_streamfile.cpp | 3 +++ src/streamfile.c | 33 ++++++++++++++++++++++++++++----- winamp/in_streamfile.c | 38 +++++++------------------------------- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/fb2k/foo_streamfile.cpp b/fb2k/foo_streamfile.cpp index 89b6463f..be5399c0 100644 --- a/fb2k/foo_streamfile.cpp +++ b/fb2k/foo_streamfile.cpp @@ -278,6 +278,9 @@ static STREAMFILE* open_foo_streamfile_buffer_by_file(service_ptr_t m_file else this_sf->file_size = 0; + /* STDIO has an optimization to close unneeded FDs if file size is less than buffer, + * but seems foobar doesn't need this (reuses FDs?) */ + return &this_sf->vt; fail: diff --git a/src/streamfile.c b/src/streamfile.c index 3859d339..0e6cb01e 100644 --- a/src/streamfile.c +++ b/src/streamfile.c @@ -94,10 +94,10 @@ static STREAMFILE* open_stdio_streamfile_buffer_by_file(FILE *infile, const char static size_t stdio_read(STDIO_STREAMFILE* sf, uint8_t* dst, offv_t offset, size_t length) { size_t read_total = 0; - if (!sf->infile || !dst || length <= 0 || offset < 0) + if (/*!sf->infile ||*/ !dst || length <= 0 || offset < 0) return 0; - //;VGM_LOG("stdio: read %lx + %x (buf %lx + %x)\n", offset, length, sf->buf_offset, sf->valid_size); + //;VGM_LOG("stdio: read %lx + %x (buf %lx + %x)\n", offset, length, sf->buf_offset, sf->valid_size, sf->buf_size); /* is the part of the requested length in the buffer? */ if (offset >= sf->buf_offset && offset < sf->buf_offset + sf->valid_size) { @@ -125,6 +125,10 @@ static size_t stdio_read(STDIO_STREAMFILE* sf, uint8_t* dst, offv_t offset, size } #endif + /* possible if all data was copied to buf and FD closed */ + if (!sf->infile) + return read_total; + /* read the rest of the requested length */ while (length > 0) { size_t length_to_read; @@ -180,12 +184,15 @@ static size_t stdio_read(STDIO_STREAMFILE* sf, uint8_t* dst, offv_t offset, size sf->offset = offset; /* last fread offset */ return read_total; } + static size_t stdio_get_size(STDIO_STREAMFILE* sf) { return sf->file_size; } + static offv_t stdio_get_offset(STDIO_STREAMFILE* sf) { return sf->offset; } + static void stdio_get_name(STDIO_STREAMFILE* sf, char* name, size_t name_size) { int copy_size = sf->name_len + 1; if (copy_size > name_size) @@ -222,7 +229,7 @@ static STREAMFILE* stdio_open(STDIO_STREAMFILE* sf, const char* const filename, /* on failure just close and try the default path (which will probably fail a second time) */ } #endif - // a normal open, open a new file + return open_stdio_streamfile_buffer(filename, buf_size); } @@ -271,13 +278,29 @@ static STREAMFILE* open_stdio_streamfile_buffer_by_file(FILE* infile, const char this_sf->file_size = 0; /* allow virtual, non-existing files */ } - /* Typically fseek(o)/ftell(o) may only handle up to ~2.14GB, signed 32b = 0x7FFFFFFF - * (happens in banks like FSB, though rarely). Should work if configured properly, log otherwise. */ + /* Typically fseek(o)/ftell(o) may only handle up to ~2.14GB, signed 32b = 0x7FFFFFFF (rarely + * happens in giant banks like FSB/KTSR). Should work if configured properly using ftell_v, log otherwise. */ if (this_sf->file_size == 0xFFFFFFFF) { /* -1 on error */ vgm_logi("STREAMFILE: file size too big (report)\n"); goto fail; /* can be ignored but may result in strange/unexpected behaviors */ } + /* Rarely a TXTP needs to open *many* streamfiles = many file descriptors = reaches OS limit = error. + * Ideally should detect better and open/close as needed or reuse FDs for files that don't play at + * the same time, but it's complex since every SF is separate (would need some kind of FD manager). + * For the time being, if the file is smaller that buffer we can just read it fully and close the FD, + * that should help since big TXTP usually just need many small files. + * Doubles as an optimization as most files given will be read fully into buf on first read. */ + if (this_sf->file_size && this_sf->file_size < this_sf->buf_size && this_sf->infile) { + //;VGM_LOG("stdio: fit filesize %x into buf %x\n", sf->file_size, sf->buf_size); + + this_sf->buf_offset = 0; + this_sf->valid_size = fread(this_sf->buf, sizeof(uint8_t), this_sf->file_size, this_sf->infile); + + fclose(this_sf->infile); + this_sf->infile = NULL; + } + return &this_sf->vt; fail: diff --git a/winamp/in_streamfile.c b/winamp/in_streamfile.c index 33652c85..a0fda4df 100644 --- a/winamp/in_streamfile.c +++ b/winamp/in_streamfile.c @@ -13,12 +13,14 @@ /* opens a utf16 (unicode) path */ static FILE* wa_fopen(const in_char* wpath) { #ifdef UNICODE_INPUT_PLUGIN - return _wfopen(wpath,L"rb"); + return _wfopen(wpath, L"rb"); #else - return fopen(wpath,"rb"); + return fopen(wpath, "rb"); #endif } +/* in theory fdopen and _wfdopen are the same, except flags is a wchar */ +#if 0 /* dupes a utf16 (unicode) file */ static FILE* wa_fdopen(int fd) { #ifdef UNICODE_INPUT_PLUGIN @@ -27,6 +29,7 @@ static FILE* wa_fdopen(int fd) { return fdopen(fd,"rb"); #endif } +#endif /* ************************************* */ /* IN_STREAMFILE */ @@ -36,7 +39,6 @@ static FILE* wa_fdopen(int fd) { typedef struct { STREAMFILE vt; STREAMFILE* stdiosf; - FILE* infile_ref; /* pointer to the infile in stdiosf (partially handled by stdiosf) */ } WINAMP_STREAMFILE; static STREAMFILE* open_winamp_streamfile_by_file(FILE* infile, const char* path); @@ -64,32 +66,7 @@ static STREAMFILE* wasf_open(WINAMP_STREAMFILE* sf, const char* const filename, if (!filename) return NULL; -#if !defined (__ANDROID__) && !defined (_MSC_VER) - /* When enabling this for MSVC it'll seemingly work, but there are issues possibly related to underlying - * IO buffers when using dup(), noticeable by re-opening the same streamfile with small buffer sizes - * (reads garbage). This reportedly causes issues in Android too */ - { - char name[PATH_LIMIT]; - sf->stdiosf->get_name(sf->stdiosf, name, PATH_LIMIT); - /* if same name, duplicate the file descriptor we already have open */ //unsure if all this is needed - if (sf->infile_ref && !strcmp(name,filename)) { - int new_fd; - FILE *new_file; - - if (((new_fd = dup(fileno(sf->infile_ref))) >= 0) && (new_file = wa_fdopen(new_fd))) { - STREAMFILE* new_sf = open_winamp_streamfile_by_file(new_file, filename); - if (new_sf) - return new_sf; - fclose(new_file); - } - if (new_fd >= 0 && !new_file) - close(new_fd); /* fdopen may fail when opening too many files */ - - /* on failure just close and try the default path (which will probably fail a second time) */ - } - } -#endif - + /* no need to wfdopen here, may use standard IO */ /* STREAMFILEs carry char/UTF8 names, convert to wchar for Winamp */ wa_char_to_ichar(wpath, PATH_LIMIT, filename); return open_winamp_streamfile_by_ipath(wpath); @@ -119,7 +96,6 @@ static STREAMFILE* open_winamp_streamfile_by_file(FILE* file, const char* path) this_sf->vt.close = (void*)wasf_close; this_sf->stdiosf = stdiosf; - this_sf->infile_ref = file; return &this_sf->vt; @@ -147,7 +123,7 @@ STREAMFILE* open_winamp_streamfile_by_ipath(const in_char* wpath) { return NULL; } - sf = open_winamp_streamfile_by_file(infile,path); + sf = open_winamp_streamfile_by_file(infile, path); if (!sf) { if (infile) fclose(infile); }