[Regression] Bring back original video quality option, handle attributes correctly#30393
[Regression] Bring back original video quality option, handle attributes correctly#30393comdev1337 wants to merge 1 commit intotelegramdesktop:devfrom
Conversation
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
2 times, most recently
from
34f96b9 to
305af70
Compare
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
from
305af70 to
d2696d6
Compare
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
from
d2696d6 to
a0026be
Compare
| auto displayHeight = now.height; | ||
| if (displayHeight >= Media::kVideoQualityOriginalOffset) { | ||
| displayHeight -= Media::kVideoQualityOriginalOffset; | ||
| } |
There was a problem hiding this comment.
Updated everything to be <79, replaced with const auto, ?:
| if (quality == _document) { | ||
| result.push_back(Media::kVideoQualityOriginalOffset + quality->resolveVideoQuality()); | ||
| } else { | ||
| result.push_back(quality->resolveVideoQuality()); | ||
| } |
There was a problem hiding this comment.
Replaced with ?:
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
3 times, most recently
from
501968b to
14dab6f
Compare
| const auto weak = base::make_weak(_widget); | ||
| crl::on_main(weak, [=] { |
There was a problem hiding this comment.
| const auto weak = base::make_weak(_widget); | |
| crl::on_main(weak, [=] { | |
| crl::on_main(_widget, [=] { |
?
| } | ||
| const auto overrideDuration = _stories | ||
| || (_chosenQuality && _chosenQuality != _document); | ||
| const auto durationDocument = (_chosenQuality && _chosenQuality != _document) |
There was a problem hiding this comment.
_chosenQuality != _document)
Does it really make sense?
There was a problem hiding this comment.
This is necessary. We do not want to rely on the provided duration attribute, which might be fake.
See: https://t.me/cursedvideofiles/7
If you open the 720p or 480p stream in 6.6.2, you'll end up with
- fake duration
- video player dimensions change according to fake w, h attributes
I aim to resolve these 2 issues:
| const auto value = (quality == _document) | ||
| ? (res + Media::kVideoQualityOriginalOffset) | ||
| : res; | ||
| if (std::find(seen.begin(), seen.end(), value) == seen.end()) { |
There was a problem hiding this comment.
| if (std::find(seen.begin(), seen.end(), value) == seen.end()) { | |
| if (ranges::find(seen, value) == seen.end()) { |
| const auto text = !quality | ||
| ? automatic | ||
| : (quality >= offset) | ||
| ? u"Original (%1p)"_q.arg(quality - offset) |
There was a problem hiding this comment.
(quality - offset)
Maybe some std::clamp, idk...
There was a problem hiding this comment.
added clamp up to 4320
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
2 times, most recently
from
a2e44ea to
9621d03
Compare
|
Made the player more resistant against incorrect width, height, duration attributes. Unfortunately, fake w, h are common.
Put some edge cases the current player doesn't account for in https://t.me/cursedvideofiles Should fix #30407 |
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
from
9621d03 to
899ded0
Compare
comdev1337
changed the title
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
from
899ded0 to
580eb8b
Compare
| } | ||
| } | ||
| const auto apiSize = isVideoFile() ? dimensions : QSize(); | ||
| const auto api = apiSize.isEmpty() |
There was a problem hiding this comment.
Such names should not be here.
There was a problem hiding this comment.
replaced these
| */ | ||
| #include "media/player/media_player_dropdown.h" | ||
|
|
||
| #include <algorithm> |
There was a problem hiding this comment.
| #include <algorithm> |
| // Propagate physical resolution parsed from the stream | ||
| to.realSize = from.realSize; |
There was a problem hiding this comment.
Physical as in the stream resolution as reported by ffmpeg, as opposed to size which is adjusted for SAR.
I can rename it to smth like "encoded resolution" if that makes more sense?
comdev1337
force-pushed
the
fix-media-player-original-quality
branch
from
580eb8b to
6153758
Compare
2 participants
Fixes #29780
Fixes #30407
Fixes: a bug (?) where the video quality text (SD/HD/4K) is not displayed on the gear button if a video has no
alt_documents(only speed options are available in the dropdown).Summary
This PR aims to help the minor subset of users with a functioning eyesight by restoring the original video quality option in the media player. The original stream is included as the second option (consistent with the ordering in the Android client).
Example video. Observe: wheels turn into slop when 1080p is selected.
tg_original_quality_h264.mp4
Example video. Handling wrong attributes correctly. We now have a 1080p stream!
tg_original_wh_fix.mp4
Implementation
Media::kVideoQualityOriginalOffset). This eliminates resolution ambiguity between an original 1080p stream and a sloppy 1080p.chooseQuality()where selecting the transcoded 1080p option selects the original 1080p, if the transcoded stream size is over the original file size, due toif (abs == closestAbs && quality->size < closestSize)realsize), do not rely on attributes whenever possible.Tried throwing a bunch of different videos at it. Works on my machine.