[FFmpeg-devel] librsvgdec: Fix frame clearing code

Submitted by Calvin Walton on Jan. 31, 2018, 10:17 p.m.

Details

Message ID 1517437057.19745.7.camel@kepstin.ca
State New
Headers show

Commit Message

Calvin Walton Jan. 31, 2018, 10:17 p.m.
The existing code attempts to clear the frame by painting in OVER mode
with transparent black - which is, of course, a no-op. As a result if
you have many input frames (e.g. you're using a sequence of svg files),
you'll start to see new frames drawn over old frames as memory gets
re-used.

Fix that by clearing the frame using the SOURCE blend mode, which
will replace whatever is in the buffer with transparent black.

Referencing the cairo FAQ,
https://www.cairographics.org/FAQ/#clear_a_surface
---
 libavcodec/librsvgdec.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

It would probably be nice to get this into 3.4 branch as well?

Comments

Rostislav Pehlivanov Feb. 1, 2018, 5:06 p.m.
On 31 January 2018 at 22:17, Calvin Walton <calvin.walton@kepstin.ca> wrote:

> The existing code attempts to clear the frame by painting in OVER mode
> with transparent black - which is, of course, a no-op. As a result if
> you have many input frames (e.g. you're using a sequence of svg files),
> you'll start to see new frames drawn over old frames as memory gets
> re-used.
>
> Fix that by clearing the frame using the SOURCE blend mode, which
> will replace whatever is in the buffer with transparent black.
>
> Referencing the cairo FAQ,
> https://www.cairographics.org/FAQ/#clear_a_surface
> ---
>  libavcodec/librsvgdec.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> It would probably be nice to get this into 3.4 branch as well?
>
> diff --git a/libavcodec/librsvgdec.c b/libavcodec/librsvgdec.c
> index e57070f8e4..e16cb6247d 100644
> --- a/libavcodec/librsvgdec.c
> +++ b/libavcodec/librsvgdec.c
> @@ -82,8 +82,11 @@ static int librsvg_decode_frame(AVCodecContext *avctx,
> void *data, int *got_fram
>
>      crender = cairo_create(image);
>
> -    cairo_set_source_rgba(crender, 0.0, 0.0, 0.0, 1.0f);
> -    cairo_paint_with_alpha(crender, 0.0f);
> +    cairo_save(crender);
> +    cairo_set_source_rgba(crender, 0.0, 0.0, 0.0, 0.0f);
> +    cairo_set_operator(crender, CAIRO_OPERATOR_SOURCE);
> +    cairo_paint(crender);
> +    cairo_restore(crender);
>
>      cairo_scale(crender, dimensions.width / (double)unscaled_dimensions.
> width,
>                  dimensions.height / (double)unscaled_dimensions.height);
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Could you resubmit the patch with CAIRO_OPERATOR_CLEAR and removing
cairo_set_source_rgba() like the docs say? We don't need generic clearing
to an arbitrary color and it might be faster if they optimize(d) it.

Thanks

Patch hide | download patch | download mbox

diff --git a/libavcodec/librsvgdec.c b/libavcodec/librsvgdec.c
index e57070f8e4..e16cb6247d 100644
--- a/libavcodec/librsvgdec.c
+++ b/libavcodec/librsvgdec.c
@@ -82,8 +82,11 @@  static int librsvg_decode_frame(AVCodecContext *avctx, void *data, int *got_fram
 
     crender = cairo_create(image);
 
-    cairo_set_source_rgba(crender, 0.0, 0.0, 0.0, 1.0f);
-    cairo_paint_with_alpha(crender, 0.0f);
+    cairo_save(crender);
+    cairo_set_source_rgba(crender, 0.0, 0.0, 0.0, 0.0f);
+    cairo_set_operator(crender, CAIRO_OPERATOR_SOURCE);
+    cairo_paint(crender);
+    cairo_restore(crender);
 
     cairo_scale(crender, dimensions.width / (double)unscaled_dimensions.width,
                 dimensions.height / (double)unscaled_dimensions.height);