diff mbox series

[FFmpeg-devel] avcodec/librsvgdec: fix memory leaks and deprecated functions

Message ID 20231008013623.292217-1-leo.izen@gmail.com
State Accepted
Commit 86ed68420d3b60439d0b7767c53d0fdc1deb7277
Headers show
Series [FFmpeg-devel] avcodec/librsvgdec: fix memory leaks and deprecated functions | expand

Commit Message

Leo Izen Oct. 8, 2023, 1:36 a.m. UTC 
At various points through the function librsvg_decode_frame, errors are
returned from immediately without deallocating any allocated structs.
This patch both fixes those leaks, and also fixes the use of functions
that are deprecated since librsvg version 2.52.0. The older calls are
still used, guarded by #ifdefs while the newer replacements are used if
librsvg >= 2.52.0. One of the deprecated functions is used as a check
for the configure shell script, so it was replaced with a different
function.

Signed-off-by: Leo Izen <leo.izen@gmail.com>
---
 configure               |  2 +-
 libavcodec/librsvgdec.c | 81 +++++++++++++++++++++++++++++++----------
 2 files changed, 63 insertions(+), 20 deletions(-)

Comments

Leo Izen Oct. 15, 2023, 12:06 p.m. UTC | #1 
On 10/7/23 21:36, Leo Izen wrote:
> At various points through the function librsvg_decode_frame, errors are
> returned from immediately without deallocating any allocated structs.
> This patch both fixes those leaks, and also fixes the use of functions
> that are deprecated since librsvg version 2.52.0. The older calls are
> still used, guarded by #ifdefs while the newer replacements are used if
> librsvg >= 2.52.0. One of the deprecated functions is used as a check
> for the configure shell script, so it was replaced with a different
> function.
> 
> Signed-off-by: Leo Izen <leo.izen@gmail.com>
> ---

Bumping, thanks.

- Leo Izen (Traneptora)
Leo Izen Oct. 22, 2023, 7:19 p.m. UTC | #2 
On 10/15/23 08:06, Leo Izen wrote:
> On 10/7/23 21:36, Leo Izen wrote:
>> At various points through the function librsvg_decode_frame, errors are
>> returned from immediately without deallocating any allocated structs.
>> This patch both fixes those leaks, and also fixes the use of functions
>> that are deprecated since librsvg version 2.52.0. The older calls are
>> still used, guarded by #ifdefs while the newer replacements are used if
>> librsvg >= 2.52.0. One of the deprecated functions is used as a check
>> for the configure shell script, so it was replaced with a different
>> function.
>>
>> Signed-off-by: Leo Izen <leo.izen@gmail.com>
>> ---
> 
> Bumping, thanks.
> 
> - Leo Izen (Traneptora)

Pushed as 86ed68420d3b.

- Leo Izen (Traneptora)
diff mbox series

Patch

diff --git a/configure b/configure
index 8a1a1b8584..25d251b389 100755
--- a/configure
+++ b/configure
@@ -6777,7 +6777,7 @@  enabled libpulse          && require_pkg_config libpulse libpulse pulse/pulseaud
 enabled librabbitmq       && require_pkg_config librabbitmq "librabbitmq >= 0.7.1" amqp.h amqp_new_connection
 enabled librav1e          && require_pkg_config librav1e "rav1e >= 0.5.0" rav1e.h rav1e_context_new
 enabled librist           && require_pkg_config librist "librist >= 0.2.7" librist/librist.h rist_receiver_create
-enabled librsvg           && require_pkg_config librsvg librsvg-2.0 librsvg-2.0/librsvg/rsvg.h rsvg_handle_render_cairo
+enabled librsvg           && require_pkg_config librsvg librsvg-2.0 librsvg-2.0/librsvg/rsvg.h rsvg_handle_new_from_data
 enabled librtmp           && require_pkg_config librtmp librtmp librtmp/rtmp.h RTMP_Socket
 enabled librubberband     && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new -lstdc++ && append librubberband_extralibs "-lstdc++"
 enabled libshaderc        && require_pkg_config spirv_compiler "shaderc >= 2019.1" shaderc/shaderc.h shaderc_compiler_initialize
diff --git a/libavcodec/librsvgdec.c b/libavcodec/librsvgdec.c
index 2f160edcdf..c328fbc774 100644
--- a/libavcodec/librsvgdec.c
+++ b/libavcodec/librsvgdec.c
@@ -38,48 +38,75 @@  static int librsvg_decode_frame(AVCodecContext *avctx, AVFrame *frame,
 {
     int ret;
     LibRSVGContext *s = avctx->priv_data;
-
-    RsvgHandle *handle;
-    RsvgDimensionData unscaled_dimensions, dimensions;
-    cairo_surface_t *image;
+    RsvgHandle *handle = NULL;
+    RsvgDimensionData dimensions;
+#if LIBRSVG_MAJOR_VERSION > 2 || LIBRSVG_MAJOR_VERSION == 2 && LIBRSVG_MINOR_VERSION >= 52
+    RsvgRectangle viewport = { 0 };
+#else
+    RsvgDimensionData unscaled_dimensions;
+#endif
+    cairo_surface_t *image = NULL;
     cairo_t *crender = NULL;
     GError *error = NULL;
+    gboolean gret;
 
     *got_frame = 0;
 
     handle = rsvg_handle_new_from_data(pkt->data, pkt->size, &error);
     if (error) {
-        av_log(avctx, AV_LOG_ERROR, "Error parsing svg!\n");
-        g_error_free(error);
-        return AVERROR_INVALIDDATA;
+        av_log(avctx, AV_LOG_ERROR, "Error parsing svg: %s\n", error->message);
+        ret = AVERROR_INVALIDDATA;
+        goto end;
     }
 
+#if LIBRSVG_MAJOR_VERSION > 2 || LIBRSVG_MAJOR_VERSION == 2 && LIBRSVG_MINOR_VERSION >= 52
+    gret = rsvg_handle_get_intrinsic_size_in_pixels(handle, &viewport.width, &viewport.height);
+    if (!gret) {
+        viewport.width = s->width ? s->width : 100;
+        viewport.height = s->height ? s->height : 100;
+    }
+    dimensions.width = (int)viewport.width;
+    dimensions.height = (int)viewport.height;
+#else
     rsvg_handle_get_dimensions(handle, &dimensions);
     rsvg_handle_get_dimensions(handle, &unscaled_dimensions);
+#endif
     dimensions.width  = s->width  ? s->width  : dimensions.width;
     dimensions.height = s->height ? s->height : dimensions.height;
     if (s->keep_ar && (s->width || s->height)) {
+#if LIBRSVG_MAJOR_VERSION > 2 || LIBRSVG_MAJOR_VERSION == 2 && LIBRSVG_MINOR_VERSION >= 52
+        double default_ar = viewport.width / viewport.height;
+#else
         double default_ar = unscaled_dimensions.width/(double)unscaled_dimensions.height;
+#endif
         if (!s->width)
             dimensions.width  = lrintf(dimensions.height * default_ar);
         else
             dimensions.height = lrintf(dimensions.width  / default_ar);
     }
 
-    if ((ret = ff_set_dimensions(avctx, dimensions.width, dimensions.height)))
-        return ret;
+    ret = ff_set_dimensions(avctx, dimensions.width, dimensions.height);
+    if (ret < 0)
+        goto end;
+
     avctx->pix_fmt = AV_PIX_FMT_RGB32;
+    viewport.width = dimensions.width;
+    viewport.height = dimensions.height;
+
+    ret = ff_get_buffer(avctx, frame, 0);
+    if (ret < 0)
+        goto end;
 
-    if ((ret = ff_get_buffer(avctx, frame, 0)))
-        return ret;
     frame->pict_type = AV_PICTURE_TYPE_I;
     frame->flags |= AV_FRAME_FLAG_KEY;
 
     image = cairo_image_surface_create_for_data(frame->data[0], CAIRO_FORMAT_ARGB32,
                                                 frame->width, frame->height,
                                                 frame->linesize[0]);
-    if (cairo_surface_status(image) != CAIRO_STATUS_SUCCESS)
-        return AVERROR_INVALIDDATA;
+    if (cairo_surface_status(image) != CAIRO_STATUS_SUCCESS) {
+        ret = AVERROR_EXTERNAL;
+        goto end;
+    }
 
     crender = cairo_create(image);
 
@@ -88,18 +115,34 @@  static int librsvg_decode_frame(AVCodecContext *avctx, AVFrame *frame,
     cairo_paint(crender);
     cairo_restore(crender);
 
+#if LIBRSVG_MAJOR_VERSION > 2 || LIBRSVG_MAJOR_VERSION == 2 && LIBRSVG_MINOR_VERSION >= 52
+    gret = rsvg_handle_render_document(handle, crender, &viewport, &error);
+#else
     cairo_scale(crender, dimensions.width / (double)unscaled_dimensions.width,
                 dimensions.height / (double)unscaled_dimensions.height);
+    gret = rsvg_handle_render_cairo(handle, crender);
+#endif
 
-    rsvg_handle_render_cairo(handle, crender);
-
-    cairo_destroy(crender);
-    cairo_surface_destroy(image);
-    g_object_unref(handle);
+    if (!gret) {
+        av_log(avctx, AV_LOG_ERROR, "Error rendering svg: %s\n", error ? error->message : "unknown error");
+        ret = AVERROR_EXTERNAL;
+        goto end;
+    }
 
     *got_frame = 1;
+    ret = 0;
 
-    return 0;
+end:
+    if (error)
+        g_error_free(error);
+    if (handle)
+        g_object_unref(handle);
+    if (crender)
+        cairo_destroy(crender);
+    if (image)
+        cairo_surface_destroy(image);
+
+    return ret;
 }
 
 #define OFFSET(x) offsetof(LibRSVGContext, x)