Update ytdl_hook.lua: Fix yt-dlp's Incomplete data received error which is rather non-critical to catch

[GunGunGun]

Because Incomplete data received isn't critical, should only check if json is blank.

Currently if you open a playlist with MPV, it's throwing this error:

mpv https://www.youtube.com/playlist?list=PLVM1qRtVEeErISdPLpYEc53XEHU5m34cl
[ytdl_hook] ERROR: Incomplete data received
[ytdl_hook] youtube-dl failed: unexpected error occurred
Failed to recognize file format.

Read this before you submit this pull request:
https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md

Reading this link and following the rules will get your pull request reviewed
and merged faster. Nobody wants lazy pull requests.

[Earnestly]

Can confirm this PR solves the very same issue I was experiencing recently; tested on commit f9234d8.

[christoph-heinrich]

yt-dlp also returns non zero status codes for other non critical errors, but some of those cases can result in not all playlist entries getting listed, and the way it is now there is no feedback that there even is something that wasn't fully successful.

Have you looked at the specific status code? Maybe we can react differently to different status codes.

Edit: Apparently the current exit codes are this and here is a related issue yt-dlp/yt-dlp#2947. There doesn't seem to have been any progress 😞

Edit2: It's a already known issue on yt-dlp's side yt-dlp/yt-dlp#8206
Also if you ever want to get this in, please have a look at the contribution guidelines for what commits should look like.

[krasnh]

This solved the problem with YouTube playlists.

[GunGunGun]

@christoph-heinrich

Have you looked at the specific status code? Maybe we can react differently to different status codes.

I debugged it quite a bit result.status = 1 in this case.

Full result logging if you want it:

[ytdl_hook] status: 1
[ytdl_hook] reason:
[ytdl_hook] stdout: {"id": "PLVM1qRtVEeErISdPLpYEc53XEHU5m34cl", "title": "Let's Play Symphony of War: Nephilim Saga", "availability": "public", "channel_follower_count": null, "description": "", "tags": [], "thumbnails": [{"url": "https://i.ytimg.com/vi/D0wjqctK9KQ/hqdefault.jpg?sqp=-[STRIPPED_CUZ_USELESS]....
[ytdl_hook]
[ytdl_hook] stderr: ERROR: Incomplete data received
[ytdl_hook]
[ytdl_hook] ERROR: Incomplete data received
[ytdl_hook] youtube-dl failed: unexpected error occurred

[christoph-heinrich]

I debugged it quite a bit result.status = 1 in this case.

No surprise there, considering the exit codes I linked.

I still don't like that the error gets completely ignored now. We should at least print a warning with stderr and status when status~=0 but the json is still usable.

[GunGunGun]

@christoph-heinrich

I still don't like that the error gets completely ignored now. We should at least print a warning with stderr and status when status~=0 but the json is still usable.

How about parsing the JSON first, I'll create a new commit and let's see if it's okay.

Actually I followed your idea and just print a warning instead.

[christoph-heinrich]

@christoph-heinrich

I still don't like that the error gets completely ignored now. We should at least print a warning with stderr and status when status~=0 but the json is still usable.

How about parsing the JSON first, I'll create a new commit and let's see if it's okay.

Actually I followed your idea and just print a warning instead.

Hmm...

at least print a warning with stderr and status

[GunGunGun]

@christoph-heinrich

at least print a warning with stderr and status

It's done.

[po5]

Read this before you submit this pull request:
https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md

[christoph-heinrich]

https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md#write-good-commit-messages
https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md#always-squash-fixup-commits-when-making-changes-to-pull-requests

The PR template literally says

Read this before you submit this pull request:
https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md

Reading this link and following the rules will get your pull request reviewed
and merged faster. Nobody wants lazy pull requests.

[GunGunGun]

@po5 @christoph-heinrich I see, I've read the guide and thanks!

[github-actions]

Download the artifacts for this pull request:

Windows

• mpv-i686-w64-mingw32
• mpv-x86_64-w64-mingw32

[Dudemanguy]

Can you please squash your commits?

[christoph-heinrich]

fwiw that specific playlist issue has already been fixed by yt-dlp. It may still make sense to allow parse-able jsons to continue, but I'm worried that it will end up leading to crashes because the json didn't contain fields that are assumed to exist by other parts of the ytdl_hook.

I don't know if such a scenario can actually happen, and technically there is always the possibility of them changing up their json structure, but it's a concern nonetheless.

[Dudemanguy]

Oh I had assumed it was another one of those "the ytdl_hook is outdated again" things. If there's no actually concrete issue this solves anymore, then I don't think we need this? All the concerns you raised are valid and if there's a use case then, we can revisit it later.