From fd5637888ac5f6be60d2aac843afe5235930214a Mon Sep 17 00:00:00 2001 From: wm4 Date: Wed, 30 Jan 2013 01:17:33 +0100 Subject: [PATCH] screenshot: minor simplification, prefer VF over VO Remove screenshot_force and associated logic. Always try to use the screenshot video filter before trying taking screenshots with the VO, which means that --vf=screenshot now takes the role of --vf=screenshot_force. (To make this clear, not adding a video filter is still the recommended way to take screenshots; we just change how VF screenshots are forced.) Preferring VO over VF and having --vf=screenshot_force used to make sense when not all VOs supported screenshots, and some VOs had somewhat broken screenshots (like vo_xv taking screenshots with OSD in it). But all these issues are fixed now, so just get rid of the cruft. --- DOCS/man/en/changes.rst | 4 +++- DOCS/man/en/mpv.rst | 18 ++++++------------ DOCS/man/en/vf.rst | 3 --- core/screenshot.c | 27 +++++---------------------- video/filter/vf.c | 2 -- video/filter/vf_screenshot.c | 11 ----------- 6 files changed, 14 insertions(+), 51 deletions(-) diff --git a/DOCS/man/en/changes.rst b/DOCS/man/en/changes.rst index 6f85c5ab38..eb3f4d0d91 100644 --- a/DOCS/man/en/changes.rst +++ b/DOCS/man/en/changes.rst @@ -53,7 +53,9 @@ General changes for mplayer2 to mpv * Make ``--softvol`` default (**mpv** is not a mixer control panel) * Improved support for .cue files * Screenshot improvements (can save screenshots as JPG or PNG, configurable - filenames, support for taking screenshots with or without subtitles) + filenames, support for taking screenshots with or without subtitles - the + ``screenshot`` video filter is not needed anymore, and should not be put + into the mpv config file) * Removal of teletext support * Replace image VOs (``--vo=jpeg`` etc.) with ``--vo=image`` * Do not lose settings when playing a new file in the same player instance diff --git a/DOCS/man/en/mpv.rst b/DOCS/man/en/mpv.rst index 73d8b0a06b..d8fa642546 100644 --- a/DOCS/man/en/mpv.rst +++ b/DOCS/man/en/mpv.rst @@ -417,19 +417,13 @@ available number - no files will be overwritten. A screenshot will usually contain the unscaled video contents at the end of the video filter chain and subtitles. By default the ``S`` takes screenshots without -subtitles. Some video output drivers will always include subtitles and OSD in -the video frame as well - this is because of technical restrictions. +subtitles, while ``s`` includes subtitles. -The ``screenshot`` video filter is normally not required when using a -recommended GUI video output driver. The ``screenshot`` filter will be attempted -to be used if the video output doesn't support screenshots. Note that taking -screenshots with the video filter is not instant: the screenshot will be only -saved when the next video frame is displayed. This means attempting to take a -screenshot while the player is paused will do nothing, until the user unpauses -or seeks. Also, the screenshot filter is not compatible with hardware decoding, -and actually will cause initialization failure when use with hardware decoding -is attempted. Using the ``screenshot`` video filter is not recommended for -these reasons. +The ``screenshot`` video filter is not required when using a recommended GUI +video output driver. It should normally not be added to the config file, as +taking screenshots is handled by the VOs, and adding the screenshot filter will +break hardware decoding. (The filter may still be useful for taking screenshots +at a certain point within the video chain when using multiple video filters.) .. include:: changes.rst diff --git a/DOCS/man/en/vf.rst b/DOCS/man/en/vf.rst index 09fb4e37c0..e993f8ec59 100644 --- a/DOCS/man/en/vf.rst +++ b/DOCS/man/en/vf.rst @@ -658,9 +658,6 @@ screenshot not always safe to insert this filter by default. See the ``Taking screenshots`` section for details. -screenshot_force - Same as ``screenshot``, but prefer it over VO based screenshot code. - sub=[=bottom-margin:top-margin] Moves subtitle rendering to an arbitrary point in the filter chain, or force subtitle rendering in the video filter as opposed to using diff --git a/core/screenshot.c b/core/screenshot.c index ae259fcbc0..e5f15948e5 100644 --- a/core/screenshot.c +++ b/core/screenshot.c @@ -272,19 +272,6 @@ static void screenshot_save(struct MPContext *mpctx, struct mp_image *image, talloc_free(image); } -static bool force_vf(struct MPContext *mpctx) -{ - if (mpctx->sh_video) { - struct vf_instance *vf = mpctx->sh_video->vfilter; - while (vf) { - if (strcmp(vf->info->name, "screenshot_force") == 0) - return true; - vf = vf->next; - } - } - return false; -} - void screenshot_request(struct MPContext *mpctx, int mode, bool each_frame) { if (mpctx->video_out && mpctx->video_out->config_ok) { @@ -307,15 +294,11 @@ void screenshot_request(struct MPContext *mpctx, int mode, bool each_frame) struct voctrl_screenshot_args args = { .full_window = (mode == MODE_FULL_WINDOW) }; - if (!force_vf(mpctx)) - vo_control(mpctx->video_out, VOCTRL_SCREENSHOT, &args); + struct vf_instance *vfilter = mpctx->sh_video->vfilter; + vfilter->control(vfilter, VFCTRL_SCREENSHOT, &args); - if (!args.out_image) { - mp_msg(MSGT_CPLAYER, MSGL_INFO, "No VO support for taking" - " screenshots, trying VFCTRL_SCREENSHOT!\n"); - struct vf_instance *vfilter = mpctx->sh_video->vfilter; - vfilter->control(vfilter, VFCTRL_SCREENSHOT, &args); - } + if (!args.out_image) + vo_control(mpctx->video_out, VOCTRL_SCREENSHOT, &args); if (args.out_image) { if (args.has_osd) @@ -323,7 +306,7 @@ void screenshot_request(struct MPContext *mpctx, int mode, bool each_frame) screenshot_save(mpctx, args.out_image, mode == MODE_SUBTITLES); } else { mp_msg(MSGT_CPLAYER, MSGL_INFO, - "...failed (need --vf=screenshot?)\n"); + "Taking screenshot failed (need --vf=screenshot?)\n"); } } } diff --git a/video/filter/vf.c b/video/filter/vf.c index 5b466031c7..719ec395fb 100644 --- a/video/filter/vf.c +++ b/video/filter/vf.c @@ -65,7 +65,6 @@ extern const vf_info_t vf_info_phase; extern const vf_info_t vf_info_divtc; extern const vf_info_t vf_info_softskip; extern const vf_info_t vf_info_screenshot; -extern const vf_info_t vf_info_screenshot_force; extern const vf_info_t vf_info_sub; extern const vf_info_t vf_info_yadif; extern const vf_info_t vf_info_stereo3d; @@ -88,7 +87,6 @@ static const vf_info_t *const filter_list[] = { #endif &vf_info_screenshot, - &vf_info_screenshot_force, &vf_info_noise, &vf_info_eq, diff --git a/video/filter/vf_screenshot.c b/video/filter/vf_screenshot.c index 8deb48924a..b28479963d 100644 --- a/video/filter/vf_screenshot.c +++ b/video/filter/vf_screenshot.c @@ -91,14 +91,3 @@ const vf_info_t vf_info_screenshot = { vf_open, NULL }; - -// screenshot.c will look for a filter named "screenshot_force", and not use -// the VO based screenshot code if it's in the filter chain. -const vf_info_t vf_info_screenshot_force = { - "screenshot to file (override VO based screenshot code)", - "screenshot_force", - "A'rpi, Jindrich Makovicka", - "", - vf_open, - NULL -};