[vlc-commits] [Git][videolan/vlc][3.0.x] 6 commits: picture: split picture_NewFromResource() in two

Steve Lhomme (@robUx4) gitlab at videolan.org
Wed Aug 26 03:40:18 UTC 2026



Steve Lhomme pushed to branch 3.0.x at VideoLAN / VLC


Commits:
ec1d10c7 by Rémi Denis-Courmont at 2026-08-26T03:33:46+00:00
picture: split picture_NewFromResource() in two

Separate the common part for resourced and non-resourced cases.

No functional changes.

(cherry picked from commit d622423f42285f04c56d51d12965cdf6120fd456)
(cherry picked from commit 0dc0949d6cdc1bfd417c9e7445a8a5dcdf6d302a)

- - - - -
5ce532ce by Rémi Denis-Courmont at 2026-08-26T03:33:46+00:00
picture: separate picture_NewFromFormat()

No need to call picture_NewFromResource() there. Just call the common
code.

No functional changes.

(cherry picked from commit da03a861eb8aeb8314a74571cb57825c83361298)
(cherry picked from commit dca0794b7812140a3c19bb3806ce74bfe98008e8)

- - - - -
2e92f36f by Rémi Denis-Courmont at 2026-08-26T03:33:46+00:00
picture: remove no-op

The unallocated picture will be destroyed immediately by the caller.
Setting the planes count to zero has no effects.

(cherry picked from commit 401bef8edf6f3e90eb245e3a6e3f1abc587359ec)
(cherry picked from commit e1c0ea5101e50a972833c011a4af16e609d5530c)

- - - - -
6e7c9db1 by Rémi Denis-Courmont at 2026-08-26T03:33:46+00:00
picture: check for overflow in picture_Setup()

(adapted from commit 50251bfbaee96d061f950dfbfc39b63797e50917)

(cherry picked from commit 2e6d8fd06dca19cd8bc13071e19c41f55ba3101e)

- - - - -
fd28daa2 by Rémi Denis-Courmont at 2026-08-26T03:33:46+00:00
picture: remove redundant test

picture_Setup() now catches conversions of large to negative earlier.

(cherry picked from commit 8e1ac9182460717a509ec17754abbd6e03e0b5b1)
(cherry picked from commit 65ee56cffeeebbe1694977edbc4ebbeacd991eb4)

- - - - -
80640fc3 by Rémi Denis-Courmont at 2026-08-26T03:33:46+00:00
picture: inline AllocatePicture() and use overflow helpers

(adapted from commits 8f5352c1182f16e88c88c87e84c4f4e37878e43e 054bcfe4a97449d57d4f701ef642fdd01b3bcca9 a96944e1c076e643b4841f52f8112790d780f33d)

(cherry picked from commit 6d858d93459ca6f7bc36830fcf03d9a0e3e5429a)

- - - - -


1 changed file:

- src/misc/picture.c


Changes:

=====================================
src/misc/picture.c
=====================================
@@ -32,67 +32,13 @@
 # include "config.h"
 #endif
 #include <assert.h>
+#include <limits.h>
 
 #include <vlc_common.h>
 #include "picture.h"
 #include <vlc_image.h>
 #include <vlc_block.h>
 
-#define PICTURE_SW_SIZE_MAX (1<<28) /* 256MB: 8K * 8K * 4*/
-
-/**
- * Allocate a new picture in the heap.
- *
- * This function allocates a fake direct buffer in memory, which can be
- * used exactly like a video buffer. The video output thread then manages
- * how it gets displayed.
- */
-static int AllocatePicture( picture_t *p_pic )
-{
-    /* Calculate how big the new image should be */
-    size_t i_bytes = 0;
-    for( int i = 0; i < p_pic->i_planes; i++ )
-    {
-        const plane_t *p = &p_pic->p[i];
-
-        if( p->i_pitch < 0 || p->i_lines <= 0 ||
-            (size_t)p->i_pitch > (SIZE_MAX - i_bytes)/p->i_lines )
-        {
-            p_pic->i_planes = 0;
-            return VLC_ENOMEM;
-        }
-        i_bytes += p->i_pitch * p->i_lines;
-    }
-
-    if( i_bytes >= PICTURE_SW_SIZE_MAX )
-    {
-        p_pic->i_planes = 0;
-        return VLC_ENOMEM;
-    }
-
-    i_bytes = (i_bytes + 63) & ~63; /* must be a multiple of 64 */
-    uint8_t *p_data = aligned_alloc( 64, i_bytes );
-    if( i_bytes > 0 && p_data == NULL )
-    {
-        p_pic->i_planes = 0;
-        return VLC_EGENERIC;
-    }
-
-    /* Fill the p_pixels field for each plane */
-    p_pic->p[0].p_pixels = p_data;
-    for( int i = 1; i < p_pic->i_planes; i++ )
-    {
-        p_pic->p[i].p_pixels = &p_pic->p[i-1].p_pixels[ p_pic->p[i-1].i_lines *
-                                                        p_pic->p[i-1].i_pitch ];
-    }
-
-    return VLC_SUCCESS;
-}
-
-/*****************************************************************************
- *
- *****************************************************************************/
-
 static void PictureDestroyContext( picture_t *p_picture )
 {
     picture_context_t *ctx = p_picture->context;
@@ -114,8 +60,7 @@ static void picture_DestroyFromResource( picture_t *p_picture )
 }
 
 /**
- * Destroys a picture allocated with picture_NewFromFormat()
- * (and thus AllocatePicture()).
+ * Destroys a picture allocated with picture_NewFromFormat().
  */
 static void picture_Destroy( picture_t *p_picture )
 {
@@ -173,9 +118,10 @@ int picture_Setup( picture_t *p_picture, const video_format_t *restrict fmt )
        Which is respected if you have
        V % lcm( p_dsc->p[0..planes].w.i_den * 16) == 0
     */
-    int i_modulo_w = 1;
-    int i_modulo_h = 1;
-    unsigned int i_ratio_h  = 1;
+    unsigned i_modulo_w = 1;
+    unsigned i_modulo_h = 1;
+    unsigned i_ratio_h  = 1;
+
     for( unsigned i = 0; i < p_dsc->plane_count; i++ )
     {
         i_modulo_w = LCM( i_modulo_w, 64 * p_dsc->p[i].w.den );
@@ -185,22 +131,44 @@ int picture_Setup( picture_t *p_picture, const video_format_t *restrict fmt )
     }
     i_modulo_h = LCM( i_modulo_h, 32 );
 
-    const int i_width_aligned  = ( fmt->i_width  + i_modulo_w - 1 ) / i_modulo_w * i_modulo_w;
-    const int i_height_aligned = ( fmt->i_height + i_modulo_h - 1 ) / i_modulo_h * i_modulo_h;
-    const int i_height_extra   = 2 * i_ratio_h; /* This one is a hack for some ASM functions */
+    unsigned width, height;
+
+    if (unlikely(add_overflow(fmt->i_width, i_modulo_w - 1, &width))
+     || unlikely(add_overflow(fmt->i_height, i_modulo_h - 1, &height)))
+        return VLC_EGENERIC;
+
+    width = width / i_modulo_w * i_modulo_w;
+    height = height / i_modulo_h * i_modulo_h;
+
+    /* Hack: append two scan lines for some SIMD assembler */
+    if (unlikely(add_overflow(height, 2 * i_ratio_h, &height)))
+        return VLC_EGENERIC;
+
+    /* plane_t uses 'int'. */
+    if (unlikely(width > INT_MAX) || unlikely(height > INT_MAX))
+        return VLC_EGENERIC;
+
     for( unsigned i = 0; i < p_dsc->plane_count; i++ )
     {
         plane_t *p = &p_picture->p[i];
+        const vlc_rational_t *h = &p_dsc->p[i].h;
+        const vlc_rational_t *w = &p_dsc->p[i].w;
+
+        /* A plane cannot be over-sampled. This could lead to overflow. */
+        assert(h->den >= h->num);
+        assert(w->den >= w->num);
 
-        p->i_lines         = (i_height_aligned + i_height_extra ) * p_dsc->p[i].h.num / p_dsc->p[i].h.den;
-        p->i_visible_lines = (fmt->i_visible_height + (p_dsc->p[i].h.den - 1)) / p_dsc->p[i].h.den * p_dsc->p[i].h.num;
-        p->i_pitch         = i_width_aligned * p_dsc->p[i].w.num / p_dsc->p[i].w.den * p_dsc->pixel_size;
-        p->i_visible_pitch = (fmt->i_visible_width + (p_dsc->p[i].w.den - 1)) / p_dsc->p[i].w.den * p_dsc->p[i].w.num * p_dsc->pixel_size;
-        p->i_pixel_pitch   = p_dsc->pixel_size;
+        p->i_lines = height * h->num / h->den;
+        p->i_visible_lines = (fmt->i_visible_height + (h->den - 1)) / h->den * h->num;
+
+        p->i_pitch = width * w->num / w->den * p_dsc->pixel_size;
+        p->i_visible_pitch = (fmt->i_visible_width + (w->den - 1)) / w->den * w->num
+                             * p_dsc->pixel_size;
+        p->i_pixel_pitch = p_dsc->pixel_size;
 
         assert( (p->i_pitch % 64) == 0 );
     }
-    p_picture->i_planes  = p_dsc->plane_count;
+    p_picture->i_planes = p_dsc->plane_count;
 
     return VLC_SUCCESS;
 }
@@ -208,7 +176,8 @@ int picture_Setup( picture_t *p_picture, const video_format_t *restrict fmt )
 /*****************************************************************************
  *
  *****************************************************************************/
-picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_resource_t *p_resource )
+
+static picture_priv_t *picture_NewPrivate(const video_format_t *restrict p_fmt)
 {
     video_format_t fmt = *p_fmt;
 
@@ -243,38 +212,82 @@ picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_r
     atomic_init( &priv->gc.refs, 1 );
     priv->gc.opaque = NULL;
 
-    if( p_resource )
-    {
-        p_picture->p_sys = p_resource->p_sys;
+    return priv;
+}
 
-        if( p_resource->pf_destroy != NULL )
-            priv->gc.destroy = p_resource->pf_destroy;
-        else
-            priv->gc.destroy = picture_DestroyFromResource;
+picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_resource_t *p_resource )
+{
+    assert(p_resource != NULL);
 
-        for( int i = 0; i < p_picture->i_planes; i++ )
-        {
-            p_picture->p[i].p_pixels = p_resource->p[i].p_pixels;
-            p_picture->p[i].i_lines  = p_resource->p[i].i_lines;
-            p_picture->p[i].i_pitch  = p_resource->p[i].i_pitch;
-        }
-    }
+    picture_priv_t *priv = picture_NewPrivate(p_fmt);
+    if (unlikely(priv == NULL))
+        return NULL;
+
+    picture_t *p_picture = &priv->picture;
+
+    p_picture->p_sys = p_resource->p_sys;
+
+    if( p_resource->pf_destroy != NULL )
+        priv->gc.destroy = p_resource->pf_destroy;
     else
+        priv->gc.destroy = picture_DestroyFromResource;
+
+    for( int i = 0; i < p_picture->i_planes; i++ )
     {
-        if( AllocatePicture( p_picture ) )
-        {
-            free( p_picture );
-            return NULL;
-        }
-        priv->gc.destroy = picture_Destroy;
+        p_picture->p[i].p_pixels = p_resource->p[i].p_pixels;
+        p_picture->p[i].i_lines  = p_resource->p[i].i_lines;
+        p_picture->p[i].i_pitch  = p_resource->p[i].i_pitch;
     }
 
     return p_picture;
 }
 
-picture_t *picture_NewFromFormat( const video_format_t *p_fmt )
+#define PICTURE_SW_SIZE_MAX (UINT32_C(1) << 28) /* 256MB: 8K * 8K * 4*/
+
+picture_t *picture_NewFromFormat(const video_format_t *restrict fmt)
 {
-    return picture_NewFromResource( p_fmt, NULL );
+    picture_priv_t *priv = picture_NewPrivate(fmt);
+    if (unlikely(priv == NULL))
+        return NULL;
+
+    priv->gc.destroy = picture_Destroy;
+
+    picture_t *pic = &priv->picture;
+    if (pic->i_planes == 0)
+        return pic;
+
+    /* Calculate how big the new image should be */
+    size_t plane_sizes[PICTURE_PLANE_MAX];
+    size_t pic_size = 0;
+
+    for (int i = 0; i < pic->i_planes; i++)
+    {
+        const plane_t *p = &pic->p[i];
+
+        if (unlikely(mul_overflow(p->i_pitch, p->i_lines, &plane_sizes[i]))
+         || unlikely(add_overflow(pic_size, plane_sizes[i], &pic_size)))
+            goto error;
+    }
+
+    if (unlikely(pic_size >= PICTURE_SW_SIZE_MAX))
+        goto error;
+
+    pic_size = (pic_size + 63) & ~63; /* must be a multiple of 64 */
+    uint8_t *buf = aligned_alloc(64, pic_size);
+    if (buf == NULL)
+        goto error;
+
+    /* Fill the p_pixels field for each plane */
+    for (int i = 0; i < pic->i_planes; i++)
+    {
+        pic->p[i].p_pixels = buf;
+        buf += plane_sizes[i];
+    }
+
+    return pic;
+error:
+    free(pic);
+    return NULL;
 }
 
 picture_t *picture_New( vlc_fourcc_t i_chroma, int i_width, int i_height, int i_sar_num, int i_sar_den )



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/91f0f51089c9460e9a1eb9f4f440a33185f84950...80640fc3f4eaf5d115aea6ed92b1d689bb020d47

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/91f0f51089c9460e9a1eb9f4f440a33185f84950...80640fc3f4eaf5d115aea6ed92b1d689bb020d47
You're receiving this email because of your account on code.videolan.org. Manage all notifications: https://code.videolan.org/-/profile/notifications | Help: https://code.videolan.org/help




More information about the vlc-commits mailing list