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

@@ -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

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
 

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

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");
         }
     }
 }

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,

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
-};