mpv
mpv copied to clipboard
various: use stream_read_file for loading files
This allows cleaning up some code duplication which can be served by stream_read_file.
Download the artifacts for this pull request:
Hmm.. I noticed just now the JS change.
It's close, but not enough.
All the js_* APIs can throw, and if js_pushlstring throws (e.g. on OOM or whatever), then the bstr is leaked.
It should use the af argument as talloc context, and without final talloc_free, because this is an auto-free function, where the af argument is freed after the function exits or throws.
@na-na-hi apologies for not noticing this earlier. Mind posting a followup fix?
With javascript plugin I get now:
[ 0.025582] file: Cannot open file '': Invalid argument
[ 0.025655] stream: Failed to open .
Is this expected?
@na-na-hi apologies for not noticing this earlier.
There's a bug in the JS commit where the semantics of af_push_file has changed. That's the comment:
// Push up to limit bytes of file fname: from builtin_files, else from the OS.
static void af_push_file(js_State *J, const char *fname, int limit, void *af)
{ ... }
Where originally it read/pushed up to limit bytes even if the file is bigger, but now with stream_read_file it errors if the file is bigger than limit.
This, in turn, breaks the require function to load JS modules, because to test if the file exists it tries to read the file with limit of 1 byte (could be improved, but for now it does that), so previously this worked, but now it fails if the file is bigger than 1 byte - as most files are.
To test it in a js script, save this e.g. as test.js:
var fname = "./js-test.txt";
mp.utils.write_file("file://" + fname, "foo bar\n");
print("size: missing:", mp.utils.read_file(fname));
print("size: -1:", mp.utils.read_file(fname, -1));
print("size: 1000:", mp.utils.read_file(fname, 1000));
print("size: 1:", mp.utils.read_file(fname, 1));
print("DONE");
then run mpv --idle --script=./test.js and see how it fails before the last line.