clsid2 / mpc-hc

Media Player Classic
GNU General Public License v3.0
11.37k stars 499 forks source link

Improve Save Image Filename for online streams #2522

Closed SomewhatDamaged closed 5 months ago

SomewhatDamaged commented 9 months ago

Issue

When taking a snapshot (Default F5) it prepends the username and password of the current RTSP stream to the filename. Example: rtsp://admin:n%7D%29%3D%23W%40V%3BKWVL%3Dd%5E%297%2ayN9%2B~sqh9Mji%29@192.168.1.140:554 admin_n})=#W@V;KWVL=d^)7_yN9+~sqh9Mji)@192.168.1_snapshot_00.02.771.jpg

Expected

Image screenshots shouldn't leak such data. Perhaps chop such stream names at the @? Example: 192.168.1_snapshot_00.02.771.jpg

System/Version Info

MPC-HC (64-bit)


Build information: Version: 2.1.4 (c82061aef) Compiler: MSVC v19.29.30152 Build date: Jan 15 2024

LAV Filters: LAV Splitter: 0.78.0.1 LAV Video: 0.78.0.1 LAV Audio: 0.78.0.1 FFmpeg compiler: MinGW-w64 GCC 13.2.0

Operating system: Name: Windows 10 (Build 22H2) Version: 10.0.19045 (64-bit)

Hardware: CPU: AMD Ryzen 7 3700X 8-Core Processor GPU1: NVIDIA GeForce GTX 1070 (driver version: 31.0.15.3623) GPU2: NVIDIA GeForce GTX 1070 (driver version: 31.0.15.3623) GPU3: NVIDIA GeForce GTX 1070 (driver version: 31.0.15.3623)

xLn2 commented 9 months ago

https://github.com/clsid2/mpc-hc/issues/995 is a similar problem that can be solved with abbreviation.

clsid2 commented 9 months ago

Maybe all online streams should just use a generic filename for screenshots. E.g. webvideo_[timestamp]_snapshot_[videotimestamp].jpg

xLn2 commented 9 months ago

I think there should be distinction between online streams (have duration) and live streams (no have duration). Since live streamings have no duration, [videotimestamp]s are somehow pointless to me. The current system time [timestamp] as a constant value makes more sense.

Online stream: Big Buck Bunny, Sunflower version Live stream: Free iptv

Online stream: Big Buck Bunny, Sunflower version Advanced settings, {SavelmagePosition, SavelmageCurrentTime opposite of default} bbb_sunflower_1080p_60fpsnormal.mp4[2024.02.05_15.23.41]

Advanced settings, {SavelmagePosition, SavelmageCurrentTime default settings} bbb_sunflower_1080p_60fps_normal.mp4_snapshot_00.15.372

Free iptv first live stream: Kanali 7 Ⓢ Advanced settings, {SavelmagePosition, SavelmageCurrentTime opposite of default} 12001.m3u8[2024.02.05_15.58.42]

Advanced settings, {SavelmagePosition, SavelmageCurrentTime default settings} 1200_1.m3u8_snapshot_10.26.495

Findings/suggestions: "snapshot" shouldn't depend on SaveImagePosition state[on]. If media has no duration just use {SavelmagePosition=False, SavelmageCurrentTime=True}. On playlists streams mostly have significant list names. For playlist files it can be used as filename. If clip name is available, it can be used as filename (Big Buck Bunny, Sunflower version has)

adipose commented 5 months ago

"snapshot" shouldn't depend on SaveImagePosition state[on].

Fixed.

If media has no duration just use {SavelmagePosition=False, SavelmageCurrentTime=True}.

Done.

On playlists streams mostly have significant list names. For playlist files it can be used as filename.

Done, but note that if it detects a youtube url, including the twitch url used in your playlist (twitch.tv/abcnewsal), it uses a different method to get the name.

If clip name is available, it can be used as filename (Big Buck Bunny, Sunflower version has)

Done, but only if save extension is disabled.

adipose commented 5 months ago

https://github.com/clsid2/mpc-hc/pull/2813

clsid2 commented 5 months ago

For local files I think it should always use the filename, not the title.

adipose commented 5 months ago

OK, added that now.