mpv icon indicating copy to clipboard operation
mpv copied to clipboard

stream_file: Get optional path field from fd protocol url.

Open YanceyChiew opened this issue 2 years ago • 2 comments

The current version of the fd and fdclose protocols can only get the file descriptor from the url, which causes path-based features like remembering the playback position and getting the title from the filename to be abnormal.

This change helps address this issue.

YanceyChiew avatar Sep 20 '22 00:09 YanceyChiew

Wait, I found a problem, the watch_later configuration file related functions get the file name too early, the changes here will not help to correctly read the last playback position once the fd changes.

YanceyChiew avatar Sep 20 '22 05:09 YanceyChiew

I'm converting this PR to a draft for now, when you fix all the problems you wanted to fix, feel free to request a review again

Traneptora avatar Sep 20 '22 12:09 Traneptora

After being reminded by a member of the mpv-android project, I realized that there is a better way to fundamentally solve the problem I encountered(Open external files using content uri on Android), without modifying mpv and with more benefits, that is to use stream_cb.

Because to solve that problem, if starting from the idea of ​​extending loadfile and stream_file, there will be a lot of changes, and its use is very narrow, I don't think it's a good idea to continue. Also, there was another PR that probably solved this problem before, unfortunately it didn't get passed.


On the other hand, relaxing the restrictions on the url format of fb and fbclose does help to solve a problem, that is, adding the path and file name to the url, at least the file title can be displayed correctly, if it is a system environment where the file descriptor can be controlled autonomously, the watch_later file can also be loaded and recorded correctly.

While it's currently possible to manually set the file's title with additional options, the benefit of having the title in the url is that it doesn't need to consider whether the media container already has a real title.

To achieve this purpose, it is only necessary to allow the file descriptor in the url to follow other characters, and the change is very small. After testing, it is unnecessary and invalid to set the filename and path of the stream.

very small change:

    if (strncmp(url, "fd://", 5) == 0 || is_fdclose) {
        char *begin = strstr(stream->url, "://") + 3, *end = NULL;
        p->fd = strtol(begin, &end, 0);
-        if (!end || end == begin || end[0]) {
+       if (!end || end == begin || (end[0] && strncmp(end, "/", 1) != 0)) {
            MP_ERR(stream, "Invalid FD: %s\n", stream->url);
            return STREAM_ERROR;
        }

So I'm thinking, should I close this pr directly, or amend the pr to this small change?

YanceyChiew avatar Sep 23 '22 04:09 YanceyChiew