From 1ef13dda12b288f8e7dbcb7448e8b02b27415515 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Wed, 28 Jun 2023 15:51:15 +0300 Subject: [PATCH] pngenc: Allocate a single GstMemory per frame Previously, we would create a new GstMemory per write operation and then append them to the GstBuffer. This would cause a reallocation every 16 Memories which is an issue since the png encoder will usually do write in a pattern of 4, 8 and 8k bytes repeating until the frame is done. Instead allocate a single GstMemory and keep writting it into it with a manual index. Much like the jpegenc does. Doing some basic testing With a testsrc snow pattern at 4k and 8k the same pipeline would take ~3.30s to encode a 4k frame and ~23s for an 8k. At 4k 0.70s/33% is taken by memory allocations, while at 8k its ~10.5s/45%. With this patch, at 4k the pipeline takes ~2.40s and at 8k only 9.60s making this 28% and 58% faster accordingly on my laptop, and allocation runtime is dropped to subsecond times. Here's the test pipeline used, increase num-buffers in image freeze to gather more samples. ``` gst-launch-1.0 videotestsrc num-buffers=1 pattern=snow ! imagefreeze num-buffers=1 ! \ video/x-raw,width=7680,height=4320 ! pngenc ! fakesink ``` Close #2717 Part-of: --- .../gst-plugins-good/ext/libpng/gstpngenc.c | 109 ++++++++++++++---- .../gst-plugins-good/ext/libpng/gstpngenc.h | 5 +- 2 files changed, 91 insertions(+), 23 deletions(-) diff --git a/subprojects/gst-plugins-good/ext/libpng/gstpngenc.c b/subprojects/gst-plugins-good/ext/libpng/gstpngenc.c index f807e4c9cd..406fe8d799 100644 --- a/subprojects/gst-plugins-good/ext/libpng/gstpngenc.c +++ b/subprojects/gst-plugins-good/ext/libpng/gstpngenc.c @@ -228,38 +228,86 @@ user_flush_data (png_structp png_ptr G_GNUC_UNUSED) { } +/* Copied from glib/gutilsprivate.h + * + * Returns the smallest power of 2 greater than or equal to n, + * or 0 if such power does not fit in a gsize + */ +static inline gsize +gst_pngenc_g_nearest_pow (gsize num) +{ + gsize n = num - 1; + + g_assert (num > 0 && num <= G_MAXSIZE / 2); + + n |= n >> 1; + n |= n >> 2; + n |= n >> 4; + n |= n >> 8; + n |= n >> 16; +#if GLIB_SIZEOF_SIZE_T == 8 + n |= n >> 32; +#endif + + return n + 1; +} + +static void +ensure_memory_is_enough (GstPngEnc * pngenc, unsigned int extra_length) +{ + GstMemory *new_memory; + GstMapInfo map; + gsize old_size, desired_size; + guint8 *new_data; + + old_size = pngenc->output_map.size; + desired_size = gst_pngenc_g_nearest_pow (old_size + extra_length); + g_assert (desired_size != 0); + + /* Our output memory wasn't big enough. + * Make a new memory that's twice the size, */ + new_memory = gst_allocator_alloc (NULL, desired_size, NULL); + gst_memory_map (new_memory, &map, GST_MAP_READWRITE); + new_data = map.data; + + /* copy previous data */ + memcpy (new_data, pngenc->output_map.data, old_size); + gst_memory_unmap (pngenc->output_mem, &pngenc->output_map); + gst_memory_unref (pngenc->output_mem); + + /* drop it into place, */ + pngenc->output_mem = new_memory; + pngenc->output_map = map; +} + static void user_write_data (png_structp png_ptr, png_bytep data, png_uint_32 length) { GstPngEnc *pngenc; - GstMemory *mem; - GstMapInfo minfo; pngenc = (GstPngEnc *) png_get_io_ptr (png_ptr); - mem = gst_allocator_alloc (NULL, length, NULL); - if (!mem) { - GST_ERROR_OBJECT (pngenc, "Failed to allocate memory"); - png_error (png_ptr, "Failed to allocate memory"); + GST_TRACE_OBJECT (pngenc, + "Memory size: %" G_GSIZE_FORMAT "\nLength to be written: %u", + pngenc->output_map.size, length); + + if (pngenc->output_map.size > G_MAXSIZE - length) { + GST_ERROR_OBJECT (pngenc, + "Memory buffer would overflow after the png write, aborting."); + png_error (png_ptr, "Buffer would overflow, aborting the write."); /* never reached */ + /* libpng will longjmp and we will catch it later on */ return; } - if (!gst_memory_map (mem, &minfo, GST_MAP_WRITE)) { - GST_ERROR_OBJECT (pngenc, "Failed to map memory"); - gst_memory_unref (mem); - - png_error (png_ptr, "Failed to map memory"); - - /* never reached */ - return; + if ((pngenc->output_mem_pos + length) > pngenc->output_map.size) { + GST_INFO_OBJECT (pngenc, "Memory not enough, Allocating more."); + ensure_memory_is_enough (pngenc, length); } - memcpy (minfo.data, data, length); - gst_memory_unmap (mem, &minfo); - - gst_buffer_append_memory (pngenc->buffer_out, mem); + memcpy (&pngenc->output_map.data[pngenc->output_mem_pos], data, length); + pngenc->output_mem_pos += length; } static GstFlowReturn @@ -269,8 +317,10 @@ gst_pngenc_handle_frame (GstVideoEncoder * encoder, GstVideoCodecFrame * frame) gint row_index; png_byte **row_pointers; GstFlowReturn ret = GST_FLOW_OK; - GstVideoInfo *info; + const GstVideoInfo *info; GstVideoFrame vframe; + gsize memory_size; + GstBuffer *outbuf; pngenc = GST_PNGENC (encoder); info = &pngenc->input_state->info; @@ -323,7 +373,15 @@ gst_pngenc_handle_frame (GstVideoEncoder * encoder, GstVideoCodecFrame * frame) } /* allocate the output buffer */ - pngenc->buffer_out = gst_buffer_new (); + pngenc->output_mem_pos = 0; + pngenc->output_mem = + gst_allocator_alloc (NULL, MAX (4096, GST_VIDEO_INFO_SIZE (info)), NULL); + if (!pngenc->output_mem) { + GST_ERROR_OBJECT (pngenc, "Failed to allocate memory"); + return GST_FLOW_ERROR; + } + + gst_memory_map (pngenc->output_mem, &pngenc->output_map, GST_MAP_READWRITE); png_write_info (pngenc->png_struct_ptr, pngenc->png_info_ptr); png_write_image (pngenc->png_struct_ptr, row_pointers); @@ -336,9 +394,16 @@ gst_pngenc_handle_frame (GstVideoEncoder * encoder, GstVideoCodecFrame * frame) png_destroy_write_struct (&pngenc->png_struct_ptr, (png_infopp) NULL); /* Set final size and store */ - frame->output_buffer = pngenc->buffer_out; + gst_memory_unmap (pngenc->output_mem, &pngenc->output_map); + /* Trim the buffer size */ + memory_size = pngenc->output_mem_pos; + gst_memory_resize (pngenc->output_mem, 0, memory_size); + pngenc->output_mem_pos = 0; - pngenc->buffer_out = NULL; + outbuf = gst_buffer_new (); + gst_buffer_append_memory (outbuf, pngenc->output_mem); + pngenc->output_mem = NULL; + frame->output_buffer = outbuf; if ((ret = gst_video_encoder_finish_frame (encoder, frame)) != GST_FLOW_OK) goto done; diff --git a/subprojects/gst-plugins-good/ext/libpng/gstpngenc.h b/subprojects/gst-plugins-good/ext/libpng/gstpngenc.h index 878038c082..d3e3f7dfad 100644 --- a/subprojects/gst-plugins-good/ext/libpng/gstpngenc.h +++ b/subprojects/gst-plugins-good/ext/libpng/gstpngenc.h @@ -37,7 +37,10 @@ struct _GstPngEnc GstVideoEncoder parent; GstVideoCodecState *input_state; - GstBuffer *buffer_out; + + GstMemory *output_mem; + GstMapInfo output_map; + gsize output_mem_pos; png_structp png_struct_ptr; png_infop png_info_ptr;