christoph-heinrich / mpv-quality-menu

A userscript for MPV that allows you to change the streamed video and audio quality (ytdl-format) on the fly.
GNU General Public License v2.0
153 stars 4 forks source link

Videos on some websites will display a lot of useless information #3

Closed dyphire closed 2 years ago

dyphire commented 2 years ago

use mpv to open https://www.bilibili.com/video/BV1234y1H7pV, then open the video menu. There will display a lot of useless information.

screenshot image

I think we can do extra processing on the obtained video format to exclude useless data that without effective resolution.

christoph-heinrich commented 2 years ago

I'm having a real hard time reproducing that. I'm using the lastest master of yt-dlp and youtube-dl. Can you send me the output of yt-dlp -J --no-warnings --flat-playlist 'https://www.bilibili.com/video/BV1234y1H7pV?p=1' please?

dyphire commented 2 years ago

I'm having a real hard time reproducing that. I'm using the lastest master of yt-dlp and youtube-dl. Can you send me the output of yt-dlp -J --no-warnings --flat-playlist 'https://www.bilibili.com/video/BV1234y1H7pV?p=1' please?

I'm using the lastest release of yt-dlp. ytdlp-log.txt

christoph-heinrich commented 2 years ago

And i think this patch should solve it in your case, but I'm not sure making it solely dependent on resolution is ideal.

diff --git a/quality-menu.lua b/quality-menu.lua
index eb4f45c..3c10879 100644
--- a/quality-menu.lua
+++ b/quality-menu.lua
@@ -321,7 +321,7 @@ local function download_formats()
             size = scale_filesize(format.filesize)
         end
         return {
-            resolution = format.resolution or string.format("%sx%s", format.width, format.height),
+            resolution = format.resolution or "",
             fps = format.fps and format.fps.."fps" or "",
             dynamic_range = format.dynamic_range or "",
             bitrate_total = scale_bitrate(format.tbr),
@@ -338,11 +338,15 @@ local function download_formats()
     end

     for i,f in ipairs(video_formats) do
-        video_formats[i].labels = format_label(f.format)
+        if f.format.resolution then
+            video_formats[i].labels = format_label(f.format)
+        end
     end

     for i,f in ipairs(audio_formats) do
-        audio_formats[i].labels = format_label(f.format)
+        if f.format.resolution then
+            audio_formats[i].labels = format_label(f.format)
+        end
     end

     local function format_table(formats, columns)
dyphire commented 2 years ago

No, this patch will not render menus on this video.

This is the error output printed by the console:

[   6.547][w][quality_menu] 
[   6.547][w][quality_menu] stack traceback:
[   6.547][w][quality_menu]     ...ools/Player/mpv/portable_config/scripts/quality-menu.lua:288: in function 'format_table'
[   6.547][w][quality_menu]     ...ools/Player/mpv/portable_config/scripts/quality-menu.lua:310: in function 'process_json'
[   6.547][w][quality_menu]     ...ools/Player/mpv/portable_config/scripts/quality-menu.lua:331: in function 'process_json_string'
[   6.547][w][quality_menu]     ...ools/Player/mpv/portable_config/scripts/quality-menu.lua:572: in function 'handler'
[   6.547][w][quality_menu]     mp.defaults:380: in function 'handler'
[   6.547][w][quality_menu]     mp.defaults:510: in function 'call_event_handlers'
[   6.547][w][quality_menu]     mp.defaults:552: in function 'dispatch_events'
[   6.547][w][quality_menu]     mp.defaults:503: in function <mp.defaults:502>
[   6.547][w][quality_menu]     [C]: at 0x7ff72d799510
[   6.547][w][quality_menu]     [C]: at 0x7ff72d797bf0
[   6.547][v][ffmpeg] Opening https://upos-sz-mirrorcoso1.bilivideo.com/upgcxcode/39/77/760877739/760877739_nb3-1-30232.m4s?e=ig8euxZM2rNcNbdlhoNvNC8BqJIzNbfqXBvEqxTEto8BTrNvN0GvT90W5JZMkX_YN0MvXg8gNEV4NC8xNEV4N03eN0B5tZlqNxTEto8BTrNvNeZVuJ10Kj_g2UB02J0mN0B5tZlqNCNEto8BTrNvNC7MTX502C8f2jmMQJ6mqF2fka1mqx6gqj0eN0B599M=&uipk=5&nbs=1&deadline=1657903059&gen=playurlv2&os=coso1bv&oi=1884344855&trid=5356a8d75d6f4bd18c59add22efb01beu&mid=0&platform=pc&upsig=013bfd17a346e2efb3f8dc88eabe45b7&uparams=e,uipk,nbs,deadline,gen,os,oi,trid,mid,platform&bvc=vod&nettype=0&orderid=0,3&agrr=1&bw=16163&logo=80000000
[   6.547][f][quality_menu] Lua error: ...ools/Player/mpv/portable_config/scripts/quality-menu.lua:266: attempt to index field 'labels' (a nil value)
dyphire commented 2 years ago

After some research, I found that m4s format files are actually cache files. The script parses these cache data and displays them on the video menu.

christoph-heinrich commented 2 years ago

Sorry I missed your post with the output of yt-dlp. This should fix it for you, but I don't like that solution. I'll figure out something better for master.

diff --git a/quality-menu.lua b/quality-menu.lua
index eb4f45c..e777e9d 100644
--- a/quality-menu.lua
+++ b/quality-menu.lua
@@ -232,7 +232,9 @@ local function download_formats()
         local is_video = (format.vcodec and format.vcodec ~= "none") or (format.video_ext and format.video_ext ~= "none")
         local is_audio = (format.acodec and format.acodec ~= "none") or (format.audio_ext and format.audio_ext ~= "none")
         if is_video then
-            video_formats[#video_formats+1] = {format=format}
+            if format.resolution then
+                video_formats[#video_formats+1] = {format=format}
+            end
         elseif is_audio and not is_video then
             audio_formats[#audio_formats+1] = {format=format}
         end
@@ -321,7 +323,7 @@ local function download_formats()
             size = scale_filesize(format.filesize)
         end
         return {
-            resolution = format.resolution or string.format("%sx%s", format.width, format.height),
+            resolution = format.resolution,
             fps = format.fps and format.fps.."fps" or "",
             dynamic_range = format.dynamic_range or "",
             bitrate_total = scale_bitrate(format.tbr),
christoph-heinrich commented 2 years ago

I've changed the video and audio detection on the dev branch. Would be great if you can test some sites with it to see if it works for you. Seems reliable based on everything I have tested so far.

dyphire commented 2 years ago

I've changed the video and audio detection on the dev branch. Would be great if you can test some sites with it to see if it works for you. Seems reliable based on everything I have tested so far.

Yes, the script of dev branch fix the issue for me.

pukkandan commented 2 years ago

I'm having a real hard time reproducing that. I'm using the lastest master of yt-dlp and youtube-dl. Can you send me the output of yt-dlp -J --no-warnings --flat-playlist 'https://www.bilibili.com/video/BV1234y1H7pV?p=1' please?

I'm using the lastest release of yt-dlp. ytdlp-log.txt

I can't reproduce this json output. @dyphire Could you open an issue on yt-dlp with the relevant verbose log?

dyphire commented 2 years ago

I can't reproduce this json output. Could you open an issue on yt-dlp with the relevant verbose log?

I don't know what issue I need to open.

pukkandan commented 2 years ago

You can use the "broken site" template. Just make sure to update yt-dlp and to give full verbose log

christoph-heinrich commented 2 years ago

Btw I "broke" this again in the latest release, and instead rely on this for deciding if its a video or audio format. If you want this fixed properly, report it here.

dyphire commented 2 years ago

Btw I "broke" this again in the latest release, and instead rely on this for deciding if its a video or audio format. If you want this fixed properly, report it here.

Thanks for reminding me. I forgot it accidentally. In addition, I think the correct change should be this:

local is_video = format.vcodec and format.vcodec ~= "none"
local is_audio = format.acodec and format.acodec ~= "none"
christoph-heinrich commented 2 years ago

When it's null, it could still be a video/audio. This certainly leads to less false positives, but it will also lead to some false negatives. Not sure which way i should go... I could also create a new option, so people can choose themselves if they want to have null codecs in their list or not.

dyphire commented 2 years ago

When it's null, it could still be a video/audio. This certainly leads to less false positives, but it will also lead to some false negatives. Not sure which way i should go...

I think we should ensure that it is video/audio, and this change can also avoid wrong formats like 'm4s', fix this isue.

At the same time, this is also the code logic in ytdl_hook.lua of the mpv official script. https://github.com/mpv-player/mpv/blob/ad5a1ac8733e6b4cf86e7b6d84c50ddf37ca62cd/player/lua/ytdl_hook.lua#L421 It is necessary to follow it first, so I don't agree with the changes in your relevant PR on https://github.com/mpv-player/mpv/pull/10428, which will introduce wrong video/audio content.

I could also create a new option, so people can choose themselves if they want to have null codecs in their list or not.

This is also an acceptable option, but the null codecs should be disabled by default.

christoph-heinrich commented 2 years ago

At the same time, this is also the code logic in ytdl_hook.lua of the mpv official script. https://github.com/mpv-player/mpv/blob/ad5a1ac8733e6b4cf86e7b6d84c50ddf37ca62cd/player/lua/ytdl_hook.lua#L421 It is necessary to follow it first, so I don't agree with the changes in your relevant PR on mpv-player/mpv#10428, which will introduce wrong video/audio content.

Yes it does that for video tracks, but it then shoves everything that isn't certainly a video into audio tracks.

I could also create a new option, so people can choose themselves if they want to have null codecs in their list or not.

This is also an acceptable option, but the null codecs should be disabled by default.

I have already implemented that before going to sleep (but didn't push it because I didn't like my wording of the description) and I think doing the same for ytdl_hook would also be a good idea. I'll revise my PR.

pukkandan commented 2 years ago

The m4s issue is with yt-dlp's bilibili extractor. And a PR to fix this is already being worked on. To determine video/audio, you only need to check for 'none' values as discussed in https://github.com/yt-dlp/yt-dlp/issues/4373.

null on the other hand just means that the codec is not known by yt-dlp and should not be excluded. If you do, the most popular sites are unlikely to be affected, but some less popular sites (which have lesser effort put into it) may fully or partially break

pukkandan commented 2 years ago

PS: The logic in ytdl_hook seem to be faulty too