Is it possible to break an EDL list to several lines?

[TCH68k]

If i have a very big EDL list and i call mpv --playlist=x.edl, then it says

[playlist] Reading plaintext playlist.
[file] error reading line
Error reading playlist 'x.edl'

Here is an example file: http://oscomp.hu/depot/x.edl

[garoto]

Here is an example file: http://oscomp.hu/depot/x.edl

that's not how an .edl file should be constructed, even retarded me knows that much.

Read https://github.com/mpv-player/mpv/blob/master/DOCS/edl-mpv.rst

And then read it again. And then again still.

[TCH68k]

Too bad, because it works with smaller files and also i was on IRC and it turned out that the error was caused by a buffer not big enough.
https://github.com/mpv-player/mpv/blob/master/demux/demux_playlist.c#L53
It will be increased to 2 MB.

BTW, I've read that manual and it said it can be splitted into lines. It did not work.

[TCH68k]

I've found a list bigger than 2 MB: https://youtube.com/watch?v=iII1nQn5DH0

[avih]

I've found a list bigger than 2 MB: https://youtube.com/watch?v=iII1nQn5DH0

Hmmm... we increased it from 512K to 2M after I explicitly asked you to assess how much should suffice - because we just don't know, and only you need this. The biggest you could find was about 520K, and we agreed 2M should be enough for the foreseeable future.

And now you say it's not enough, and I believe you, but we can't just increase it every few weeks. Using a dynamic buffer instead of a static one should solve it, but it needs to be assessed (it definitely wouldn't be as simple) - especially when we're talking about the limit for a single input line.

Not sure what to do about it.

[TCH68k]

First, not only i needed it, my users also needed it, who use MPV as a video player for my YT frontend.

Second, i actually no longer need it, as i've already made a workaround for it: http://oscomp.hu/?details/EDL-Preloader_1.0.0_(EDL_preloader_for_MPV)_(CP)_1591
I still use an older MPV with the 512kB limit and without problems. I just wanted to inform you, that 2 MB did not prove to be big enough, so the problem still exists.

Third, we did not agree at all. I had several suggestion, like 4 MB or make it configurable, but you said no. I've just accepted your decision. IRClog:

[23:40:29] <avih> we first need some estimate of what sizes we need to deal with. changing it to a dynamic buffer is more work, more LOC, and more potential for bugs. so if we can avoid it, it's better. but if it turns out that edl files can be 100M, then we need to rethink it. so we need estimations of sizes in practice
[23:40:43] <TCH680x0> 100M not
[23:40:51] <avih> and also i need to first understand exactly how this limit is applied, and how come it doesn't limit the internal edl content
[23:41:00] <TCH680x0> that would be days of content
[23:41:16] <TCH680x0> but several megs it can i think
[23:41:34] <TCH680x0> but how about a static buffer
[23:41:43] <avih> well, the one you linked is 512k and it's only 1 hour, right?
[23:41:44] <TCH680x0> which canbe configured?
[23:41:49] <TCH680x0> yes
[23:41:55] <TCH680x0> but i doubt this is linear
[23:42:03] <avih> configuration is also a PITA. that's not a solution for something which the user shouldn't care about.
[23:42:15] <TCH680x0> got it
[23:42:43] <avih> configuration is a hack which says "we don't know how to automate it, so we'll pass on this headache to the user". that's not a solution.
[23:43:32] <TCH680x0> if we extrapolate the size linearly, then a full day long video would be 12 MB
[23:43:35] <avih> either we have an estimate and we use a big enough limit (until the next time we raise it), or it's dynamic if it can be huge. so that's why we need an estimation
[23:43:59] <TCH680x0> but this is also depends on resolution
[23:44:16] <avih> so, instead of guessing, can you do some actual testing?
[23:44:20] <TCH680x0> i tried to search already
[23:44:32] <TCH680x0> but i found no 4k/fullHD videos with adaptive formats
[23:44:43] <TCH680x0> youtube only recently started to split the videos
[23:45:14] <avih> so, do some research, and report back the biggest edl file you can end up with. i can double it, but i prefer to first know the actual sizes in practice.
[23:45:15] <TCH680x0> so not every video is available in these chunks
[23:45:31] <TCH680x0> why don't you just set it to 4 MB?
[23:45:42] * Quits: Ivii (~Ivyy@2001:a61:13eb:f801:e004:51a6:2a21:ea07�) (Quit: Leaving�)
[23:45:45] <TCH680x0> that is not that big
[23:45:54] * Joins: Ivii (~Ivyy@2001:a61:13eb:f801:e004:51a6:2a21:ea07)
[23:45:56] <avih> because if the biggest you can find is 512k then it's a waste of ram
[23:46:20] <avih> do some legwork.
[23:46:27] <TCH680x0> we already found one bigger than 512k
[23:46:37] <avih> you want to raise the limit, give us some actual numbers.
[23:46:40] <TCH680x0> so 512k is definetely not enough
[23:46:45] <TCH680x0> okay
[23:46:56] <avih> sure. but we need numbers to know what IS a good limit.
[23:52:00] * Parts: ius (~ius@wireguard/tunneler/ius)
[23:52:46] <TCH680x0> it seems that very few videos have these lists
[23:53:04] <TCH680x0> i am looking for big videos and no luck
[23:57:40] <TCH680x0> okay
[23:57:44] <TCH680x0> how about 768k?
[23:58:05] <avih> later. first, give me the biggest one you can find.
[23:58:19] <TCH680x0> i found no bigger
[23:58:49] <avih> how many videos did you try?
[23:59:01] <TCH680x0> several dozens
[23:59:13] <avih> and roughly what were the sizes?
[23:59:35] <TCH680x0> 200k to 550k
[23:59:42] <avih> k, thanks.
[23:59:52] <avih> then i think 2M would be ok.
[23:59:53] <TCH680x0> the biggest was 568k IIRC
[00:00:33] <TCH680x0> thanks

Fourth, it's not my fault, that i could not find a video with EDLs bigger than 2 MB in mere minutes.

As for the dynamic buffer, here is a made up patch: http://oscomp.hu/depot/demux_playlist.diff
I just checked demux_playlist.c and the dependencies of the corresponding parts and modified the static line reading to an ever-extending one. Did not tested it (i don't have a working MPV building environment here), just had thrown it together.

[jeeb]

Yup, a dynamic buffer sounds like the least bad alternative, but low level manual allocation should probably be kept away from on this level. I'd believe we have string helpers to keep it simple such as bstr_xappend or mp_append_utf8_bstr that utilize talloc buffers in the background.

Alternatively, we look into the EDL definition to enable it being on more than one line, since the buffer is meant to be "just for a single line" xD .

[TCH68k]

Well, allowing EDLs to be broken up to multiple lines would work for me (i can break up EDLs in my generator), but the original problem would be still remain: gigantic one lined EDLs cannot be loaded by MPV.

[avih]

gigantic one lined EDLs cannot be loaded by MPV.

It can, and did/does at ytdl_hook.lua even before we increased the limit. However, IIRC this is done via a property, while the limit is applied when reading lines from a file on disk.

Well, allowing EDLs to be broken up to multiple lines would work

Yeah, but I'm not familiar with the EDL handling code, and I don't know whether it would still be considered valid (I don't know if there's some EDL spec which requires the segments to be at one line).

At the time increasing the limit seemed reasonable and we assumed it would hold for long enough, therefore we didn't look into the EDL handling code.

If it can be modified to support split-lines EDL, without breaking some specification, then it would be the best solution - instead of allowing infinitely long EDL lines.

But this requires more work which someone would need to do...

[TCH68k]

It can, and did/does at ytdl_hook.lua even before we increased the limit.

I meant via the --playlist option, not via Lua extensions. This ticket was all about the --playlist option.

EDL specs says, that multi-line EDL is valid: we have to replace the semicolons with linefeeds. (MPV does not support this, i tried.)

How about simply making the linebuffer dynamic? For that, there is a patch already.

[avih]

How about simply making the linebuffer dynamic? For that, there is a patch already.

Unless this the only correct solution - which I don't know right now (and it would require effort to know), then to me this feels like a hack, which is why I prefer to not go through with this.

If another mpv developer wants to take over and make it happen, no objection here.