[vlc-devel] [PATCH v2] vout: snapshot: fix snapshots cropping

[Romain Vimont]

The snapshot picture was created using picture_Copy(), which does not
handle offsets. As a consequence, snapshots of cropped videos were
broken.

Therefore, add a function to easily copy the region of a picture into a
new picture_t, and use it to create the snapshot.

Fixes <https://trac.videolan.org/vlc/ticket/18970>.

Signed-off-by: Romain Vimont <rom at rom1v.com>
---
CHANGES from v1:
 - rename 'picture_CloneRegion' to 'picture_CopyRegion'
 - do not export the function (VLC_API) anymore
 - move its declaration from vlc_picture.h to picture.h

 src/misc/image.c            |  5 +++++
 src/misc/picture.c          | 39 +++++++++++++++++++++++++++++++++++++++
 src/misc/picture.h          | 13 +++++++++++++
 src/video_output/snapshot.c |  8 +++++---
 4 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/src/misc/image.c b/src/misc/image.c
index aa5dd43a2d..7c3ad995c8 100644
--- a/src/misc/image.c
+++ b/src/misc/image.c
@@ -778,6 +778,11 @@ static filter_t *CreateFilter( vlc_object_t *p_this, const es_format_t *p_fmt_in
     es_format_Copy( &p_filter->fmt_in, p_fmt_in );
     es_format_Copy( &p_filter->fmt_out, p_fmt_in );
     video_format_Copy( &p_filter->fmt_out.video, p_fmt_out );
+
+    /* whatever the input offset, write at offset 0 in the target image */
+    p_filter->fmt_out.video.i_x_offset = 0;
+    p_filter->fmt_out.video.i_y_offset = 0;
+
     p_filter->fmt_out.i_codec = p_fmt_out->i_chroma;
     p_filter->p_module = module_need( p_filter, "video converter", NULL, false );
 
diff --git a/src/misc/picture.c b/src/misc/picture.c
index 59d0aaa359..1e00e04bb3 100644
--- a/src/misc/picture.c
+++ b/src/misc/picture.c
@@ -381,6 +381,45 @@ void picture_Copy( picture_t *p_dst, const picture_t *p_src )
     picture_CopyProperties( p_dst, p_src );
 }
 
+picture_t *picture_CopyRegion( const picture_t *p_src,
+                               int i_x_offset, int i_y_offset,
+                               int i_width, int i_height )
+{
+    /* allocate image with target height to avoid waste,
+     * but use the source width (with margins) to call memcpy() only once */
+    picture_t *p_dst = picture_New( p_src->format.i_chroma,
+                                    p_src->format.i_width,
+                                    i_height,
+                                    p_src->format.i_sar_num,
+                                    p_src->format.i_sar_den );
+    if( !p_dst )
+        return NULL;
+
+    picture_CopyProperties( p_dst, p_src );
+
+    /* only the left and right margins are kept in the target picture */
+    p_dst->format.i_visible_width = i_width;
+    p_dst->format.i_x_offset = i_x_offset;
+    p_dst->format.i_y_offset = 0;
+
+    const vlc_chroma_description_t *p_dsc =
+        vlc_fourcc_GetChromaDescription( p_src->format.i_chroma );
+    if( !p_dsc )
+        return NULL;
+
+    for( int i = 0; i < p_src->i_planes; ++i )
+    {
+        /* add (den - 1) to get the result rounded up */
+        int i_lines = (i_height + p_dsc->p[i].h.den - 1)
+                    * p_dsc->p[i].h.num / p_dsc->p[i].h.den;
+        int i_lines_offset = i_y_offset * p_dsc->p[i].h.num / p_dsc->p[i].h.den;
+        memcpy( p_dst->p[i].p_pixels,
+                p_src->p[i].p_pixels + i_lines_offset * p_src->p[i].i_pitch,
+                i_lines * p_src->p[i].i_pitch );
+    }
+
+    return p_dst;
+}
 
 /*****************************************************************************
  *
diff --git a/src/misc/picture.h b/src/misc/picture.h
index cf511139c9..b0ebe7fcbf 100644
--- a/src/misc/picture.h
+++ b/src/misc/picture.h
@@ -31,3 +31,16 @@ typedef struct
         void *opaque;
     } gc;
 } picture_priv_t;
+
+/**
+ * This function will create a new picture containing the requested region.
+ *
+ * \param p_src pointer to the source picture.
+ * \param i_x_offset the region x offset.
+ * \param i_y_offset the region x offset.
+ * \param i_width the region with.
+ * \param i_height the region height.
+ */
+picture_t *picture_CopyRegion( const picture_t *p_src,
+                               int i_x_offset, int i_y_offset,
+                               int i_width, int i_height );
diff --git a/src/video_output/snapshot.c b/src/video_output/snapshot.c
index ceade008cc..09cdd0c6cb 100644
--- a/src/video_output/snapshot.c
+++ b/src/video_output/snapshot.c
@@ -38,6 +38,7 @@
 #include <vlc_block.h>
 #include <vlc_vout.h>
 
+#include "misc/picture.h"
 #include "snapshot.h"
 #include "vout_internal.h"
 
@@ -119,12 +120,13 @@ void vout_snapshot_Set(vout_snapshot_t *snap,
 
     vlc_mutex_lock(&snap->lock);
     while (snap->request_count > 0) {
-        picture_t *dup = picture_NewFromFormat(fmt);
+        picture_t *dup = picture_CopyRegion( picture, fmt->i_x_offset,
+                                                      fmt->i_y_offset,
+                                                      fmt->i_visible_width,
+                                                      fmt->i_visible_height );
         if (!dup)
             break;
 
-        picture_Copy(dup, picture);
-
         dup->p_next = snap->picture;
         snap->picture = dup;
         snap->request_count--;
-- 
2.11.0