vo_xv: don't require frame stepping to remove OSD or subs

In order to improve performance, vo_xv didn't create a backup of the
video frame before drawing OSD and subtitles during normal playback. It
required the frontend to do frame stepping if it wanted to redraw the
OSD, but no backup of the video frame was available. (Consider the
following use case: enable the OSD permanently with --osd-level=3, then
pause during playback and do something that shows an OSD message. The
player will advance the video by one frame at the time the new OSD
message is first drawn.)

This also meant that taking a screenshot during playback with vo_xv
would include OSD and subtitles in the resulting image.

Fix this by always creating a backup before drawing OSD or subtitles.
In order to avoid having to create a full copy of the whole image frame,
introduce a complex scheme that tries to backup only the changed
regions.

It's unclear whether the additional complexity in draw_bmp.c for
backing up only the changed areas of the frame is worth it. Possibly
a simpler implementation would suffice, such as tracking only Y ranges
of changed image data, or even just copying the full frame.

vo_xv's get_screenshot() now always creates a copy in order not to
modify the currently displayed frame.

5 files changed, 173 insertions, 36 deletions

diff --git a/sub/draw_bmp.c b/sub/draw_bmp.c
index e578bd0fcf..96d87eec0c 100644
--- a/sub/draw_bmp.c
+++ b/sub/draw_bmp.c
@@ -374,6 +374,7 @@ static void align_bbox(int xstep, int ystep, struct mp_rect *rc)
     rc->y1 = FFALIGN(rc->y1, ystep);
 }
 
+// Post condition, if true returned: rc is inside img
 static bool align_bbox_for_swscale(struct mp_image *img, struct mp_rect *rc)
 {
     struct mp_rect img_rect = {0, 0, img->w, img->h};
@@ -527,4 +528,127 @@ void mp_draw_sub_bitmaps(struct mp_draw_sub_cache **cache, struct mp_image *dst,
     }
 }
 
+struct mp_draw_sub_backup
+{
+    bool valid;
+    struct mp_image *image;                     // backed up image parts
+    struct line_ext *lines[MP_MAX_PLANES];      // backup range for each line
+};
+
+struct line_ext {
+    int x0, x1; // x1 is exclusive
+};
+
+struct mp_draw_sub_backup *mp_draw_sub_backup_new(void)
+{
+    return talloc_zero(NULL, struct mp_draw_sub_backup);
+}
+
+// Signal that the full image is valid (nothing to backup).
+void mp_draw_sub_backup_reset(struct mp_draw_sub_backup *backup)
+{
+    backup->valid = true;
+    if (backup->image) {
+        for (int p = 0; p < MP_MAX_PLANES; p++) {
+            int h = backup->image->h;
+            for (int y = 0; y < h; y++) {
+                struct line_ext *ext = &backup->lines[p][y];
+                ext->x0 = ext->x1 = -1;
+            }
+        }
+    }
+}
+
+static void backup_realloc(struct mp_draw_sub_backup *backup,
+                           struct mp_image *img)
+{
+    if (backup->image && backup->image->imgfmt == img->imgfmt
+        && backup->image->w == img->w && backup->image->h == img->h)
+        return;
+
+    talloc_free_children(backup);
+    backup->image = alloc_mpi(img->w, img->h, img->imgfmt);
+    talloc_steal(backup, backup->image);
+    for (int p = 0; p < MP_MAX_PLANES; p++) {
+        backup->lines[p] = talloc_array(backup, struct line_ext,
+                                        backup->image->h);
+    }
+    mp_draw_sub_backup_reset(backup);
+}
+
+static void copy_line(struct mp_image *dst, struct mp_image *src,
+                      int p, int plane_y, int x0, int x1)
+{
+    int bits = MP_IMAGE_BITS_PER_PIXEL_ON_PLANE(dst, p);
+    int xs = p ? dst->chroma_x_shift : 0;
+    memcpy(dst->planes[p] + plane_y * dst->stride[p] + (x0 >> xs) * bits / 8,
+           src->planes[p] + plane_y * src->stride[p] + (x0 >> xs) * bits / 8,
+           ((x1 - x0) >> xs) * bits / 8);
+}
+
+static void backup_rect(struct mp_draw_sub_backup *backup, struct mp_image *img,
+                        int plane, struct mp_rect rc)
+{
+    if (!align_bbox_for_swscale(img, &rc))
+        return;
+    int ys = plane ? img->chroma_y_shift : 0;
+    int yp = ys ? ((1 << ys) - 1) : 0;
+    for (int y = (rc.y0 >> ys); y < ((rc.y1 + yp) >> ys); y++) {
+        struct line_ext *ext = &backup->lines[plane][y];
+        if (ext->x0 == -1) {
+            copy_line(backup->image, img, plane, y, rc.x0, rc.x1);
+            ext->x0 = rc.x0;
+            ext->x1 = rc.x1;
+        } else {
+            if (rc.x0 < ext->x0) {
+                copy_line(backup->image, img, plane, y, rc.x0, ext->x0);
+                ext->x0 = rc.x0;
+            }
+            if (ext->x1 < rc.x1) {
+                copy_line(backup->image, img, plane, y, ext->x1, rc.x1);
+                ext->x1 = rc.x1;
+            }
+        }
+    }
+}
+
+void mp_draw_sub_backup_add(struct mp_draw_sub_backup *backup,
+                            struct mp_image *img, struct sub_bitmaps *sbs)
+{
+    backup_realloc(backup, img);
+
+    for (int p = 0; p < img->num_planes; p++) {
+        for (int i = 0; i < sbs->num_parts; ++i) {
+            struct sub_bitmap *sb = &sbs->parts[i];
+            struct mp_rect rc = {sb->x, sb->y, sb->x + sb->dw, sb->y + sb->dh};
+            backup_rect(backup, img, p, rc);
+        }
+    }
+}
+
+bool mp_draw_sub_backup_restore(struct mp_draw_sub_backup *backup,
+                                struct mp_image *buffer)
+{
+    if (!backup->image || backup->image->imgfmt != buffer->imgfmt
+        || backup->image->w != buffer->w || backup->image->h != buffer->h
+        || !backup->valid)
+    {
+        backup->valid = false;
+        return false;
+    }
+    struct mp_image *img = backup->image;
+    for (int p = 0; p < img->num_planes; p++) {
+        int ys = p ? img->chroma_y_shift : 0;
+        int yp = ys ? ((1 << ys) - 1) : 0;
+        int p_h = ((img->h + yp) >> ys);
+        for (int y = 0; y < p_h; y++) {
+            struct line_ext *ext = &backup->lines[p][y];
+            if (ext->x0 < ext->x1) {
+                copy_line(buffer, img, p, y, ext->x0, ext->x1);
+            }
+        }
+    }
+    return true;
+}
+
 // vim: ts=4 sw=4 et tw=80
diff --git a/sub/draw_bmp.h b/sub/draw_bmp.h
index 489e91f666..2eae68b58c 100644
--- a/sub/draw_bmp.h
+++ b/sub/draw_bmp.h
@@ -12,6 +12,14 @@ void mp_draw_sub_bitmaps(struct mp_draw_sub_cache **cache, struct mp_image *dst,
 
 extern const bool mp_draw_sub_formats[SUBBITMAP_COUNT];
 
+struct mp_draw_sub_backup;
+struct mp_draw_sub_backup *mp_draw_sub_backup_new(void);
+void mp_draw_sub_backup_add(struct mp_draw_sub_backup *backup,
+                            struct mp_image *img, struct sub_bitmaps *sbs);
+void mp_draw_sub_backup_reset(struct mp_draw_sub_backup *backup);
+bool mp_draw_sub_backup_restore(struct mp_draw_sub_backup *backup,
+                                struct mp_image *buffer);
+
 #endif /* MPLAYER_DRAW_BMP_H */
 
 // vim: ts=4 sw=4 et tw=80
diff --git a/sub/sub.c b/sub/sub.c
index 8fde350d4d..dd8c887b4c 100644
--- a/sub/sub.c
+++ b/sub/sub.c
@@ -262,6 +262,7 @@ void osd_draw(struct osd_state *osd, struct mp_osd_res res,
 struct draw_on_image_closure {
     struct osd_state *osd;
     struct mp_image *dest;
+    struct mp_draw_sub_backup *bk;
     bool changed;
 };
 
@@ -269,6 +270,8 @@ static void draw_on_image(void *ctx, struct sub_bitmaps *imgs)
 {
     struct draw_on_image_closure *closure = ctx;
     struct osd_state *osd = closure->osd;
+    if (closure->bk)
+        mp_draw_sub_backup_add(closure->bk, closure->dest, imgs);
     mp_draw_sub_bitmaps(&osd->draw_cache, closure->dest, imgs);
     talloc_steal(osd, osd->draw_cache);
     closure->changed = true;
@@ -284,6 +287,15 @@ bool osd_draw_on_image(struct osd_state *osd, struct mp_osd_res res,
     return closure.changed;
 }
 
+void osd_draw_on_image_bk(struct osd_state *osd, struct mp_osd_res res,
+                          double video_pts, int draw_flags,
+                          struct mp_draw_sub_backup *bk, struct mp_image *dest)
+{
+    struct draw_on_image_closure closure = {osd, dest, bk};
+    osd_draw(osd, res, video_pts, draw_flags, mp_draw_sub_formats,
+             &draw_on_image, &closure);
+}
+
 void vo_osd_changed(int new_value)
 {
     struct osd_state *osd = global_osd;
diff --git a/sub/sub.h b/sub/sub.h
index f894682596..205aa953d7 100644
--- a/sub/sub.h
+++ b/sub/sub.h
@@ -223,6 +223,11 @@ struct mp_image;
 bool osd_draw_on_image(struct osd_state *osd, struct mp_osd_res res,
                        double video_pts, int draw_flags, struct mp_image *dest);
 
+struct mp_draw_sub_backup;
+void osd_draw_on_image_bk(struct osd_state *osd, struct mp_osd_res res,
+                          double video_pts, int draw_flags,
+                          struct mp_draw_sub_backup *bk, struct mp_image *dest);
+
 struct mp_rect;
 bool sub_bitmaps_bb(struct sub_bitmaps *imgs, struct mp_rect *out_bb);
 
diff --git a/video/out/vo_xv.c b/video/out/vo_xv.c
index d171559f53..3e749a5f2b 100644
--- a/video/out/vo_xv.c
+++ b/video/out/vo_xv.c
@@ -47,9 +47,11 @@
 #include "vo.h"
 #include "video/vfcap.h"
 #include "video/mp_image.h"
+#include "video/filter/vf.h"
 #include "x11_common.h"
 #include "video/memcpy_pic.h"
 #include "sub/sub.h"
+#include "sub/draw_bmp.h"
 #include "aspect.h"
 #include "video/csputils.h"
 #include "core/subopt-helper.h"
@@ -69,10 +71,9 @@ struct xvctx {
     int current_ip_buf;
     int num_buffers;
     int total_buffers;
-    bool have_image_copy;
-    bool unchanged_image;
     int visible_buf;
-    XvImage *xvimage[2 + 1];
+    XvImage *xvimage[2];
+    struct mp_draw_sub_backup *osd_backup;
     uint32_t image_width;
     uint32_t image_height;
     uint32_t image_format;
@@ -83,7 +84,7 @@ struct xvctx {
     uint32_t max_width, max_height; // zero means: not set
     int mode_switched;
 #ifdef HAVE_SHM
-    XShmSegmentInfo Shminfo[2 + 1];
+    XShmSegmentInfo Shminfo[2];
     int Shmem_Flag;
 #endif
 };
@@ -150,7 +151,6 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
     }
 
     ctx->visible_buf = -1;
-    ctx->have_image_copy = false;
 
     /* check image formats */
     ctx->xv_format = 0;
@@ -210,7 +210,7 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
         deallocate_xvimage(vo, i);
 
     ctx->num_buffers = 2;
-    ctx->total_buffers = ctx->num_buffers + 1;
+    ctx->total_buffers = ctx->num_buffers;
 
     for (i = 0; i < ctx->total_buffers; i++)
         allocate_xvimage(vo, i);
@@ -334,14 +334,6 @@ static struct mp_image get_xv_buffer(struct vo *vo, int buf_index)
     return img;
 }
 
-static void copy_backup_image(struct vo *vo, int dest, int src)
-{
-    struct mp_image img_dest = get_xv_buffer(vo, dest);
-    struct mp_image img_src = get_xv_buffer(vo, src);
-
-    copy_mpi(&img_dest, &img_src);
-}
-
 static void check_events(struct vo *vo)
 {
     int e = vo_x11_check_events(vo);
@@ -371,22 +363,17 @@ static void draw_osd(struct vo *vo, struct osd_state *osd)
         .video_par = vo->aspdat.par,
     };
 
-    if (osd_draw_on_image(osd, res, osd->vo_pts, 0, &img))
-        ctx->unchanged_image = false;
+    osd_draw_on_image_bk(osd, res, osd->vo_pts, 0, ctx->osd_backup, &img);
 }
 
 static int redraw_frame(struct vo *vo)
 {
     struct xvctx *ctx = vo->priv;
 
-    if (ctx->have_image_copy)
-        copy_backup_image(vo, ctx->visible_buf, ctx->num_buffers);
-    else if (ctx->unchanged_image) {
-        copy_backup_image(vo, ctx->num_buffers, ctx->visible_buf);
-        ctx->have_image_copy = true;
-    }  else
-        return false;
+    struct mp_image img = get_xv_buffer(vo, ctx->visible_buf);
+    mp_draw_sub_backup_restore(ctx->osd_backup, &img);
     ctx->current_buf = ctx->visible_buf;
+
     return true;
 }
 
@@ -433,6 +420,8 @@ static int draw_slice(struct vo *vo, uint8_t *image[], int stride[], int w,
     else
         memcpy_pic(dst, image[2], w, h, current_image->pitches[1], stride[2]);
 
+    mp_draw_sub_backup_reset(ctx->osd_backup);
+
     return 0;
 }
 
@@ -440,21 +429,22 @@ static mp_image_t *get_screenshot(struct vo *vo)
 {
     struct xvctx *ctx = vo->priv;
 
+    struct mp_image img = get_xv_buffer(vo, ctx->visible_buf);
+    struct mp_image *res = alloc_mpi(img.w, img.h, img.imgfmt);
+    copy_mpi(res, &img);
+    vf_clone_mpi_attributes(res, &img);
     // try to get an image without OSD
-    int id = ctx->have_image_copy ? ctx->num_buffers : ctx->visible_buf;
-    struct mp_image img = get_xv_buffer(vo, id);
-    img.display_w = vo->aspdat.prew;
-    img.display_h = vo->aspdat.preh;
+    mp_draw_sub_backup_restore(ctx->osd_backup, res);
+    res->display_w = vo->aspdat.prew;
+    res->display_h = vo->aspdat.preh;
 
-    return talloc_memdup(NULL, &img, sizeof(img));
+    return res;
 }
 
 static uint32_t draw_image(struct vo *vo, mp_image_t *mpi)
 {
     struct xvctx *ctx = vo->priv;
 
-    ctx->have_image_copy = false;
-
     if (mpi->flags & MP_IMGFLAG_DRAW_CALLBACK)
         ; // done
     else if (mpi->flags & MP_IMGFLAG_PLANAR)
@@ -468,11 +458,8 @@ static uint32_t draw_image(struct vo *vo, mp_image_t *mpi)
     else
           return false;
 
-    if (ctx->is_paused) {
-        copy_backup_image(vo, ctx->num_buffers, ctx->current_buf);
-        ctx->have_image_copy = true;
-    }
-    ctx->unchanged_image = true;
+    mp_draw_sub_backup_reset(ctx->osd_backup);
+
     return true;
 }
 
@@ -635,6 +622,8 @@ static int preinit(struct vo *vo, const char *arg)
     ctx->fo = XvListImageFormats(x11->display, x11->xv_port,
                                  (int *) &ctx->formats);
 
+    ctx->osd_backup = talloc_steal(ctx, mp_draw_sub_backup_new());
+
     return 0;
 
   error:
@@ -695,7 +684,6 @@ static int control(struct vo *vo, uint32_t request, void *data)
     case VOCTRL_SCREENSHOT: {
         struct voctrl_screenshot_args *args = data;
         args->out_image = get_screenshot(vo);
-        args->has_osd = !ctx->have_image_copy;
         return true;
     }
     }