[vlc-commits] [Git][videolan/vlc][master] 2 commits: misc: image: refactor and fix ImageWrite

Jean-Baptiste Kempf (@jbk) gitlab at videolan.org
Sun Jan 15 21:11:28 UTC 2023



Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
2ea2c7eb by Alexandre Janniaux at 2023-01-15T20:03:37+00:00
misc: image: refactor and fix ImageWrite

Refactor code to always call the encoder at the end and separate the
filtering case.

Fix a crash (double-free) when the image handler needs to resize the
picture before encoding it.

    ==150429==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000002bb8 at pc 0x7f4ce1a8a092 bp 0x7fff205294c0 sp 0x7fff205294b0
    WRITE of size 8 at 0x614000002bb8 thread T0
        #0 0x7f4ce1a8a091 in vlc_atomic_rc_dec ../../include/vlc_atomic.h:66
        #1 0x7f4ce1a8a091 in picture_Release ../../include/vlc_picture.h:372
        #2 0x7f4ce1a8a091 in ImageWrite ../../src/misc/image.c:454
        #3 0x564f4f6a00f7 in OpenIntf ../../test/src/misc/image.c:52
        #4 0x7f4ce177bfee in generic_start ../../src/modules/modules.c:275
        #5 0x7f4ce177db75 in vlc_module_load ../../src/modules/modules.c:243
        #6 0x7f4ce177df33 in module_need ../../src/modules/modules.c:286
        #7 0x7f4ce179cbdd in intf_Create ../../src/interface/interface.c:172
        #8 0x7f4ce179d86a in libvlc_InternalAddIntf ../../src/interface/interface.c:267
        #9 0x7f4ce2350b22 in libvlc_add_intf ../../lib/playlist.c:41
        #10 0x564f4f69f53c in main ../../test/src/misc/image.c:122

- - - - -
2e8f5eb5 by Alexandre Janniaux at 2023-01-15T20:03:37+00:00
test: add test for src/misc/image.c

The test tries to write the image with a custom encoder and custom
converter, executing first a fmt_in = fmt_out request, then requesting a
fmt_out with size / 2 to trigger the converter code path.

Fix regression on ImageWrite leading to a crash from previous commit.

- - - - -


3 changed files:

- src/misc/image.c
- test/Makefile.am
- + test/src/misc/image.c


Changes:

=====================================
src/misc/image.c
=====================================
@@ -378,8 +378,6 @@ static block_t *ImageWrite( image_handler_t *p_image, picture_t *p_pic,
                             const video_format_t *p_fmt_in,
                             const video_format_t *p_fmt_out )
 {
-    block_t *p_block;
-
     /* Check if we can reuse the current encoder */
     if( p_image->p_enc &&
         ( p_image->p_enc->fmt_out.i_codec != p_fmt_out->i_chroma ||
@@ -404,7 +402,6 @@ static block_t *ImageWrite( image_handler_t *p_image, picture_t *p_pic,
         p_image->p_enc->fmt_in.video.i_height != p_fmt_in->i_height ||
        !BitMapFormatIsSimilar( &p_image->p_enc->fmt_in.video, p_fmt_in ) )
     {
-        picture_t *p_tmp_pic;
 
         if( p_image->p_converter &&
             ( p_image->p_converter->fmt_in.video.i_chroma != p_fmt_in->i_chroma ||
@@ -442,25 +439,18 @@ static block_t *ImageWrite( image_handler_t *p_image, picture_t *p_pic,
             es_format_Copy( &p_image->p_converter->fmt_out, &p_image->p_enc->fmt_in );
         }
 
-        picture_Hold( p_pic );
-
-        p_tmp_pic =
-            p_image->p_converter->ops->filter_video( p_image->p_converter, p_pic );
+        p_pic = p_image->p_converter->ops->filter_video( p_image->p_converter, p_pic );
 
-        if( likely(p_tmp_pic != NULL) )
+        if( likely(p_pic != NULL) )
         {
-            assert(!picture_HasChainedPics(p_tmp_pic)); // no chaining
-            p_block = vlc_encoder_EncodeVideo(p_image->p_enc, p_tmp_pic );
-            picture_Release( p_tmp_pic );
+            assert(!picture_HasChainedPics(p_pic)); // no chaining
         }
-        else
-            p_block = NULL;
-    }
-    else
-    {
-        p_block = vlc_encoder_EncodeVideo( p_image->p_enc, p_pic );
     }
 
+    block_t *p_block = NULL;
+    if (p_pic != NULL)
+        p_block = vlc_encoder_EncodeVideo(p_image->p_enc, p_pic);
+
     if( !p_block )
     {
         msg_Dbg( p_image->p_parent, "no image encoded" );


=====================================
test/Makefile.am
=====================================
@@ -35,6 +35,7 @@ check_PROGRAMS = \
 	test_src_misc_bits \
 	test_src_misc_epg \
 	test_src_misc_keystore \
+	test_src_misc_image \
 	test_src_video_output \
 	test_src_video_output_opengl \
 	test_modules_packetizer_helpers \
@@ -200,6 +201,9 @@ test_src_input_decoder_SOURCES = \
 	src/input/decoder/input_decoder_scenarios.c
 test_src_input_decoder_LDADD = $(LIBVLCCORE) $(LIBVLC)
 
+test_src_misc_image_SOURCES = src/misc/image.c
+test_src_misc_image_LDADD = $(LIBVLCCORE) $(LIBVLC)
+
 checkall:
 	$(MAKE) check_PROGRAMS="$(check_PROGRAMS) $(EXTRA_PROGRAMS)" check
 


=====================================
test/src/misc/image.c
=====================================
@@ -0,0 +1,161 @@
+/*****************************************************************************
+ * image.c: test for the image_handler code from vlc_image.h
+ *****************************************************************************
+ * Copyright (C) 2023 Videolabs
+ *
+ * Authors: Alexandre Janniaux <ajanni at videolabs.io>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+/* Define a builtin module for mocked parts */
+#define MODULE_NAME test_misc_image
+#define MODULE_STRING "test_misc_image"
+#undef __PLUGIN__
+const char vlc_module_name[] = MODULE_STRING;
+
+#include "../../libvlc/test.h"
+
+#include <vlc/vlc.h>
+
+#include <vlc_common.h>
+#include <vlc_image.h>
+#include <vlc_picture.h>
+#include <vlc_plugin.h>
+#include <vlc_block.h>
+#include <vlc_codec.h>
+#include <vlc_filter.h>
+
+#include <limits.h>
+
+static int OpenIntf(vlc_object_t *root)
+{
+    image_handler_t *ih = image_HandlerCreate(root);
+    assert(ih != NULL);
+
+    video_format_t fmt_in;
+    video_format_Init(&fmt_in, VLC_CODEC_RGBA);
+    fmt_in.i_width = fmt_in.i_visible_width = 800;
+    fmt_in.i_height = fmt_in.i_visible_height = 600;
+
+    video_format_t fmt_out;
+    video_format_Init(&fmt_out, VLC_CODEC_PNG);
+    fmt_out.i_width = fmt_out.i_visible_width = 800;
+    fmt_out.i_height = fmt_out.i_visible_height = 600;
+
+    picture_t *picture = picture_NewFromFormat(&fmt_in);
+    assert(picture != NULL);
+
+    block_t *block;
+
+    block = image_Write(ih, picture, &fmt_in, &fmt_out);
+    assert(block != NULL);
+    block_Release(block);
+
+    picture = picture_NewFromFormat(&fmt_in);
+    fmt_out.i_width = fmt_out.i_visible_width = 400;
+    fmt_out.i_height = fmt_out.i_visible_height = 300;
+    block = image_Write(ih, picture, &fmt_in, &fmt_out);
+    assert(block != NULL);
+
+    block_Release(block);
+    image_HandlerDelete(ih);
+
+    return VLC_SUCCESS;
+}
+
+static block_t * EncodeVideo(encoder_t *encoder, picture_t *pic)
+{
+    (void)encoder;
+
+    /* Dummy encoder */
+    picture_Release(pic);
+    return block_Alloc(1);
+}
+
+static int OpenEncoder(vlc_object_t *obj)
+{
+    encoder_t *encoder = (encoder_t*)obj;
+    static const struct vlc_encoder_operations ops =
+    {
+        .encode_video = EncodeVideo
+    };
+    encoder->ops = &ops;
+    return VLC_SUCCESS;
+}
+
+static picture_t *ConvertVideo(filter_t *filter, picture_t *pic)
+{
+    picture_Release(pic);
+    return picture_NewFromFormat(&filter->fmt_out.video);
+}
+
+static int OpenConverter(vlc_object_t *obj)
+{
+    filter_t *filter = (filter_t*)obj;
+
+    static const struct vlc_filter_operations ops =
+    {
+        .filter_video = ConvertVideo,
+    };
+    filter->ops = &ops;
+    return VLC_SUCCESS;
+}
+
+/** Inject the mocked modules as a static plugin: **/
+vlc_module_begin()
+    set_callback(OpenEncoder)
+    set_capability("video encoder", INT_MAX)
+
+    add_submodule()
+        set_callback(OpenConverter)
+        set_capability("video converter", INT_MAX)
+
+    add_submodule()
+        set_callback(OpenIntf)
+        set_capability("interface", 0)
+
+vlc_module_end()
+
+/* Helper typedef for vlc_static_modules */
+typedef int (*vlc_plugin_cb)(vlc_set_cb, void*);
+
+VLC_EXPORT const vlc_plugin_cb vlc_static_modules[] = {
+    VLC_SYMBOL(vlc_entry),
+    NULL
+};
+
+
+int main()
+{
+    test_init();
+
+    const char * const args[] = {
+        "-vvv", "--vout=dummy", "--aout=dummy", "--text-renderer=dummy",
+        "--no-auto-preparse",
+    };
+
+    libvlc_instance_t *vlc = libvlc_new(ARRAY_SIZE(args), args);
+
+    libvlc_add_intf(vlc, MODULE_STRING);
+    libvlc_playlist_play(vlc);
+
+    libvlc_release(vlc);
+
+}



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/c91fac9b59449671b8135205e4975d3ee819ef7d...2e8f5eb5bf301f2cfd4040b0b036ab0037375e72

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/c91fac9b59449671b8135205e4975d3ee819ef7d...2e8f5eb5bf301f2cfd4040b0b036ab0037375e72
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list