This needs to be documented, see how #10410 did that.

/cc @christoph-heinrich

Has the situation changed since #10410, which was described as "never going to land"?

Also this PR doesn't address the following:
#12852 (comment)

This is only set when ytdl_hook ran at least once, how useful is that for other scripts?

#10410 (comment)

Such raw data cannot be relied upon by scripts, as it can change at any time. For instance if tomorrow ytdl-hook invokes ytdl twice, or if tomorrow it's invoked in a way where the JSON doesn't have the other tracks info (because ytdl-hook doesn't actually need the info for other tracks when not using all-formats=yes), etc.

This is only set when ytdl_hook ran at least once, how useful is that for other scripts?

Chicken and egg problem. Someone has to detect and set the path of ytdl. And ytdl_hook is the entrypoint for all ytld operations, so imho it is expected that all "child" scripts that may depend on this information should run only if the value is set.

Such raw data cannot be relied upon by scripts, as it can change at any time. For instance if tomorrow ytdl-hook invokes ytdl twice, or if tomorrow it's invoked in a way where the JSON doesn't have the other tracks info (because ytdl-hook doesn't actually need the info for other tracks when not using all-formats=yes), etc.

This is the question for script makers. If this info is useful. It is not "raw data" it is json with defined structure upstream. If any info is not included in the result the "child' script can act accordingly. I think the above comment is fucusing on some specific use-case with all-formats mention. But in fact having json output from ytdl might be used in various ways and if user is adding a "child" script they should also make sure that ytdl-hook config is correct.

If the alternative is to rerun youtube-dl in every script that might need that info, I think the solution of sharing the json is better.

Download the artifacts for this pull request:

Someone has to detect and set the path of ytdl.

This is only needed if the scripts want to run the ytdl binary themselves, right? In this case why can't the path detection function be extracted and made as a common function available to scripts?

If the alternative is to rerun youtube-dl in every script that might need that info, I think the solution of sharing the json is better.

Agreed, but this still needs to be documented as such, which means mpv passes ytdl's output as-is.

This is only needed if the scripts want to run the ytdl binary themselves, right? In this case why can't the path detection function be extracted and made as a common function available to scripts?

Currently we don't detect path in separate step. Script just try to run the subprocess and if it succeeded it means we "have" ytdl. This is only a problem if user has custom path set in options.

This is only a problem if user has custom path set in options.

In this case scripts can just read that option directly.

I don't think they can. Also there is still some detection.

This is only needed if the scripts want to run the ytdl binary themselves, right? In this case why can't the path detection function be extracted and made as a common function available to scripts?

The ytdl binary search can indeed be extracted into a separate function, and even placed in a separate script if desired. I extracted ytdl_hook's ytdl path detection in one of my "library" scripts that my other scripts use. Doing so in a user script is inefficient though, as my understanding is that the result can't be cached among different user scripts, every user script that requires that "library" script has its own instance of it. Now, if it was provided my mpv, like you are proposing, then I imagine the result of searching for the ytdl binary could be cached among multiple user scripts?

Has the situation changed since #10410, which was described as "never going to land"?

My understanding for the "never going to land" comment is that a big part of that was the PR being essentially the XY problem (XY solution?).

This is only set when ytdl_hook ran at least once, how useful is that for other scripts?

Can't speak for everyone, everyone's scripts are different, but it is very useful for me. My scripts act on the input file/URL, so they run ytdl only when mp.get_property("path") is a URL, like YouTube, which is the case when the ytdl binary path gets set by ytdl_hook, so my scripts can use it.

Anyway, the intention of the PR is to simply make things that yt_hook already has, available to other scripts, in case they find them useful, with the minimal amount of changes and technical dept. If they find them useful -- good, if not, then the user scripts are not worse than where they were before this PR.

Such raw data cannot be relied upon by scripts, as it can change at any time. For instance if tomorrow ytdl-hook invokes ytdl twice, or if tomorrow it's invoked in a way where the JSON doesn't have the other tracks info (because ytdl-hook doesn't actually need the info for other tracks when not using all-formats=yes), etc.

The yt-dlp json format stability is obviously at the whim of the yt-dlp project, who can break it if they so desire. So user-data/ytdl/json must be documented as such, as an unstable/uncontrollable format as far as mpv is concerned. Since mpv can't promise the stability of the json format, mpv / ytdl_hook breaking it shouldn't be much of an issue.

After playing around with yt-dlp, I noticed that when it errors, it doesn't produce json output with the error information, it only prints the error to stderr. For example, if a Twitch channel channel_name is offline, you get:

ERROR: [twitch:stream] channel_name: The channel is not currently live

and no json.

So if user scripts wanted to react to a specific yt-dlp error, they can't do that with the json output alone as there would be no json output in the case of an error, the scripts also need to access the stderr output, checking for error patterns, to make sense of what has happened. It upsets me that yt-dlp doesn't provide error information in a machine-readable format like json, but it is what it is. So I'm thinking of changing the PR to export the entire subprocess return value, the table containing: stdout, stderr, status, killed_by_us, etc., as a single native property (not as a script-message as script-messages can send only strings, which subprocess's return value isn't). That way user scripts have all the information they can possibly have, the json, the errors, the exit code, etc.

Modified ytdl_hook.lua to do the aforementioned from my last comment -- making the entire result of the ytdl --json-dump subprocess command available to user scripts, i.e. the stdout, the stderr and the exit code.

Also added some documentation. Please check if the placement (being nested under user-data) and the wording is alright.

Should the url be added to the result? Then scripts can check that the result belongs to the file that they think it belongs to, after all they run asynchronously.

Maybe I'm overly paranoid here, as I'm sure this will work fine pretty much all of the time anyway.