various: sync track information output #14453

guidocella · 2024-06-27T13:42:45Z

Print "Image" instead of "Video" for video tracks. This fixes #8561.

Print the hls-bitrate in select.lua.

Show the same flags in loadfile.c, select.lua and stats.lua. The only differences are that only stats.lua prints albumart because it's supposed to show detailed track information, and select.lua prints the image flag because pressing g-v doesn't show Video or Image like in loadfile.c and stats.lua.

github-actions · 2024-06-27T13:55:42Z

Download the artifacts for this pull request:

Windows

macOS

na-na-hi · 2024-06-27T16:43:12Z

Print "Image" instead of "Video" for video tracks. This fixes #8561.

This isn't an accurate description of images:

yes/true if this is a video track that consists of a single picture, no/false or unavailable otherwise. The heuristic used to determine if a stream is an image doesn't attempt to detect images in codecs normally used for videos.

The behavior is also different for different demuxers. For example, for gif animations, demux_lavf considers it a video, while demux_mf considers it an image. demux_lavf also considers a single frame AV1 video as an image even though it's a video in all other aspects.

kasper93 · 2024-06-27T16:46:28Z

single frame AV1 video as an image

single frame AV1 is basically an AVIF image. It is correct to call it image.

kasper93 · 2024-06-27T16:47:08Z

player/lua/stats.lua

-    local flags = {"default", "forced", "external", "dependent",
-                   "hearing-impaired", "visual-impaired", "image", "albumart"}
+    local flags = {"default", "forced", "dependent", "visual-impaired",
+                   "hearing-impaired", "albumart", "external"}


I would still leave image flag here, even if redundant.

player/lua/select.lua

guidocella · 2024-06-27T16:54:56Z

This isn't an accurate description of images:

yes/true if this is a video track that consists of a single picture, no/false or unavailable otherwise. The heuristic used to determine if a stream is an image doesn't attempt to detect images in codecs normally used for videos.

The behavior is also different for different demuxers. For example, for gif animations, demux_lavf considers it a video, while demux_mf considers it an image. demux_lavf also considers a single frame AV1 video as an image even though it's a video in all other aspects.

I know, I wrote that paragraph. ;) I indeed explicitly programmed demux_lavf to consider 1-frame gifs as images as I have several actual old images with gif codec for some reason. Gif videos are detected as videos by demux_lavf and do not normally go through demux_mf since ffmpeg can detect them. And I made demux_lavf detect 1-frame AV1 videos as images because avif are indeed real images. Anyway, the image detection is reliable outside of FATE. I wouldn't use it to prevent video playback like in the previous gainmap PR but it's fine for nicer textual description.

kasper93 · 2024-06-27T17:22:17Z

player/lua/select.lua

@@ -79,15 +98,16 @@ local function format_track(track)
             and track["codec-profile"] .. " " or "") ..
            (track["demux-samplerate"] and track["demux-samplerate"] / 1000 ..
             " kHz " or "") ..
-            (track.external and "external " or "")
-        ):sub(1, -2) .. ")"
+            (track["hls-bitrate"] and track["hls-bitrate"] .. " kbps " or "")


There is also demux-bitrate if you want to use that.

Fixes mpv-player#8561.

Show the same flags in loadfile.c, select.lua and stats.lua. The only differences are that only stats.lua prints both image and albumart because it's supposed to show detailed track information, and select.lua prints the image flag because pressing g-v doesn't show Video or Image like in loadfile.c and stats.lua.

kasper93 · 2024-06-27T21:11:27Z

player/loadfile.c

-    if (s && s->hls_bitrate > 0)
-        APPEND(b, " %d kbps", (s->hls_bitrate + 500) / 1000);
+    if (s && s->codec->bitrate)
+        APPEND(b, " %d KiBps", s->codec->bitrate / 8 / 1000);


1024, but see other comments first.

na-na-hi · 2024-06-27T21:06:34Z

DOCS/interface-changes/packet-bitrate.txt

@@ -0,0 +1,2 @@
+remove deprecated `packet-video-bitrate` `packet-audio-bitrate` and
+`packet-sub-bitrate` properties


No line breaks. The lines are wrapped automatically by the generation scripts.

na-na-hi · 2024-06-27T21:12:26Z

player/command.c

        } else {
-            *(char **)arg = talloc_asprintf(NULL, "%.3f Mbps", rate / 1000.0);
+            *(char **)arg = talloc_asprintf(NULL, "%.3f MiBps", rate / 1024);


This shouldn't be changed. Are there any software or websites even using these units?

kasper93 · 2024-06-27T21:20:38Z

player/command.c

        } else {
-            *(char **)arg = talloc_asprintf(NULL, "%.3f Mbps", rate / 1000.0);
+            *(char **)arg = talloc_asprintf(NULL, "%.3f MiBps", rate / 1024);


IMHO KiB/s looks less cursed than KiBps.

But it also more common to use bits per second. So maybe we should stick to that, to avoid angry users coming at us. And kilobits per seconds, not kibibits.

I know I suggested using utils.format_bytes_humanized, but it seems to do more harm than good. Unless we actually change there.

So to summary. I would use bits, kb/s, Mb/s for bitrates. That would be compatible with common values that other software show.

EDIT:

We can add, to be used with bits values, like bitrate. And keep using the other one for cache size. Dunno.

function mp_utils.format_bits_humanized(b) local d = {"bits", "kb", "Mb", "Gb", "Tb", "Pb"} local i = 1 while b >= 1000 do b = b / 1000 i = i + 1 end return string.format("%0.2f %s", b, d[i] and d[i] or "*1000^" .. (i-1)) end

na-na-hi · 2024-06-28T15:24:09Z

player/loadfile.c

+    if (s && s->codec->bitrate)
+        APPEND(b, " %d kbps", (s->codec->bitrate + 500) / 1000);
+    if (s && s->hls_bitrate)
        APPEND(b, " %d kbps", (s->hls_bitrate + 500) / 1000);


I think it looks confusing to have two bitrates printed without any indications.

Added "HLS" to hls-bitrate only since it's the only that's totally inaccurate.

These have been deprecated for 9 years so it's fine to remove them. Using the replacement properties like video-bitrate in stats.lua will convert big enough bitrates to Mbps.

kasper93 · 2024-07-17T20:14:46Z

player/loadfile.c

+    if (s && s->codec->bitrate)
+        APPEND(b, " %d kbps", (s->codec->bitrate + 500) / 1000);
+    if (s && s->hls_bitrate)
+        APPEND(b, " %d HLS kbps", (s->hls_bitrate + 500) / 1000);


this looks strange to me

kasper93

Looks good, apart from HLS naming, looks odd.

sfan5 · 2024-08-01T17:26:09Z

player/loadfile.c

+    if (s && s->codec->bitrate)
+        APPEND(b, " %d kbps", (s->codec->bitrate + 500) / 1000);
+    if (s && s->hls_bitrate)
+        APPEND(b, " %d HLS kbps", (s->hls_bitrate + 500) / 1000);


what kind of unit is "HLS kbps"?
I get the idea to indicate where the bitrate came from but it looks weird and I don't think users really care.

my suggestion is to print codec bitrate if available or else print hls bitrate, but without the extra "HLS"
(based on the assumption that codec bitrate is generally more accurate than hls bitrate)

Print demux-bitrate if available, else hls-bitrate, not both (as demux-bitrate is generally more reliable). This avoids printing "HLS kbps" which looks weird. Fixes mpv-player#14453 (comment)

Print demux-bitrate if available, else hls-bitrate, not both (as demux-bitrate is generally more reliable). This avoids printing "HLS kbps" which looks weird. Fixes #14453 (comment)

guidocella force-pushed the track-info branch 3 times, most recently from 646b914 to b8738e6 Compare June 27, 2024 15:33

kasper93 reviewed Jun 27, 2024

View reviewed changes

player/lua/select.lua Show resolved Hide resolved

guidocella force-pushed the track-info branch from b8738e6 to 8f7bac4 Compare June 27, 2024 17:16

kasper93 reviewed Jun 27, 2024

View reviewed changes

guidocella force-pushed the track-info branch from 8f7bac4 to c8955e4 Compare June 27, 2024 18:00

guidocella added 2 commits June 27, 2024 21:28

various: print Image instead or Video for image tracks

5fc2ad2

Fixes mpv-player#8561.

guidocella force-pushed the track-info branch from c8955e4 to c2bf757 Compare June 27, 2024 20:22

kasper93 reviewed Jun 27, 2024

View reviewed changes

na-na-hi reviewed Jun 27, 2024

View reviewed changes

kasper93 reviewed Jun 27, 2024

View reviewed changes

guidocella force-pushed the track-info branch from c2bf757 to 1d9754c Compare June 28, 2024 07:10

na-na-hi reviewed Jun 28, 2024

View reviewed changes

guidocella added 2 commits June 28, 2024 17:37

loadfile,select.lua: print bitrates of tracks

eb0af02

command: remove deprecated packet-*-bitrate properties

21a87ef

These have been deprecated for 9 years so it's fine to remove them. Using the replacement properties like video-bitrate in stats.lua will convert big enough bitrates to Mbps.

guidocella force-pushed the track-info branch from 1d9754c to 21a87ef Compare June 28, 2024 15:38

kasper93 reviewed Jul 17, 2024

View reviewed changes

kasper93 approved these changes Jul 17, 2024

View reviewed changes

kasper93 merged commit 8110bda into mpv-player:master Jul 18, 2024
21 checks passed

guidocella deleted the track-info branch July 18, 2024 20:53

sfan5 reviewed Aug 1, 2024

View reviewed changes

guidocella mentioned this pull request Aug 1, 2024

loadfile,select.lua: print only one bitrate #14621

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

various: sync track information output #14453

various: sync track information output #14453

guidocella commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

na-na-hi commented Jun 27, 2024

kasper93 commented Jun 27, 2024 •

edited

Loading

kasper93 Jun 27, 2024

guidocella Jun 27, 2024

guidocella commented Jun 27, 2024

kasper93 Jun 27, 2024

guidocella Jun 27, 2024

kasper93 Jun 27, 2024 •

edited

Loading

na-na-hi Jun 27, 2024

na-na-hi Jun 27, 2024

kasper93 Jun 27, 2024 •

edited

Loading

na-na-hi Jun 28, 2024

guidocella Jun 28, 2024

kasper93 Jul 17, 2024

kasper93 left a comment

sfan5 Aug 1, 2024 •

edited

Loading

		@@ -0,0 +1,2 @@
		remove deprecated `packet-video-bitrate` `packet-audio-bitrate` and
		`packet-sub-bitrate` properties

various: sync track information output #14453

various: sync track information output #14453

Conversation

guidocella commented Jun 27, 2024 • edited Loading

github-actions bot commented Jun 27, 2024 • edited Loading

na-na-hi commented Jun 27, 2024

kasper93 commented Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guidocella commented Jun 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kasper93 Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kasper93 Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kasper93 left a comment

Choose a reason for hiding this comment

sfan5 Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

guidocella commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

kasper93 commented Jun 27, 2024 •

edited

Loading

kasper93 Jun 27, 2024 •

edited

Loading

kasper93 Jun 27, 2024 •

edited

Loading

sfan5 Aug 1, 2024 •

edited

Loading