[vlc-commits] [Git][videolan/vlc][master] 3 commits: image: ensure picture is released from client
Felix Paul Kühne (@fkuehne)
gitlab at videolan.org
Thu Feb 2 15:17:38 UTC 2023
Felix Paul Kühne pushed to branch master at VideoLAN / VLC
Commits:
49a1f114 by Alexandre Janniaux at 2023-02-02T14:45:59+00:00
image: ensure picture is released from client
Ensure the client calling the export to block_t is also responsible for
releasing the picture it exports.
Previous commits were adding the release to match behaviour of the
converter before refactoring everything, but it was a mistake and led to
issues with other parts of the pipeline, in particular the transcoder,
and some clients were more adapted to the converter path + hold of
original rather than the non-conversion path, which would then have been
simpler to do from the start.
This MR adds the necessary hold and release to remove those from the
encoders and the clients, and is tested against the unmerged
libvlc/media.c test for thumbnailing too.
Revert "png: fix picture release on encoder"
This reverts commit d4c93a29a223c6d68e3336f5312a03644896e09e.
Revert "jpeg: fix picture release on encoder"
This reverts commit cccc085e30d6ca4862f43bf93372e2fc156c1778.
Revert "avcodec: encoder: fix picture release"
This reverts commit 5a145f34fa463ac458e65e9f1b2a2e2595c58494.
Revert "codec: vpx: fix picture release on encoder"
This reverts commit 5dde7db39c1c9fd1f72b28dc1786798300b6106c.
Revert "image: fix picture leak in case of error"
This reverts commit 1bf09fe8d6ca2b7d90d48b29f8d0f2164a84d382.
Revert "misc: thumbnailer: release picture when interrupted"
This reverts commit 004b9900f837147d49895e96f86ae2c1ef4c3dea.
- - - - -
66f60e1a by Alexandre Janniaux at 2023-02-02T14:45:59+00:00
image: merge if() condition and assert
The if block is only used to trigger the assert now, so merge it into
the assert itself.
- - - - -
46ca013b by Alexandre Janniaux at 2023-02-02T14:45:59+00:00
test: image: check that encoder opened
The test can pass if a different encoder is used during the test, but we
need to ensure it's really the encoder from the test.
This was catched when changing the capability from video encoder into
image encoder, which led the test to use the png encoder instead.
- - - - -
10 changed files:
- modules/codec/avcodec/encoder.c
- modules/codec/jpeg.c
- modules/codec/png.c
- modules/codec/vpx.c
- modules/misc/medialibrary/Thumbnailer.cpp
- src/input/thumbnailer.c
- src/misc/image.c
- test/src/input/thumbnail.c
- test/src/misc/image.c
- test/src/misc/image_cvpx.c
Changes:
=====================================
modules/codec/avcodec/encoder.c
=====================================
@@ -1280,7 +1280,6 @@ static block_t *EncodeVideo( encoder_t *p_enc, picture_t *p_pict )
}
block_t *p_block = encode_avframe( p_enc, p_sys, frame );
- picture_Release( p_pict );
return p_block;
}
=====================================
modules/codec/jpeg.c
=====================================
@@ -651,7 +651,6 @@ static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
block_t *p_block = block_Alloc(p_sys->i_blocksize);
if (p_block == NULL)
{
- picture_Release(p_pic);
return NULL;
}
@@ -736,7 +735,6 @@ static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
p_block->i_buffer = size;
p_block->i_dts = p_block->i_pts = p_pic->date;
- picture_Release(p_pic);
return p_block;
@@ -753,7 +751,6 @@ error:
free(p_row_pointers);
block_Release(p_block);
- picture_Release(p_pic);
return NULL;
}
=====================================
modules/codec/png.c
=====================================
@@ -446,7 +446,6 @@ static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
block_t *p_block = block_Alloc( p_sys->i_blocksize );
if( p_block == NULL )
{
- picture_Release( p_pic );
return NULL;
}
@@ -454,7 +453,6 @@ static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
if( p_png == NULL )
{
block_Release( p_block );
- picture_Release( p_pic );
return NULL;
}
@@ -521,7 +519,6 @@ static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
p_block->i_buffer = i_start - p_block->i_buffer;
p_block->i_dts = p_block->i_pts = p_pic->date;
- picture_Release( p_pic );
return p_block;
@@ -530,6 +527,5 @@ static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
png_destroy_write_struct( &p_png, p_info ? &p_info : NULL );
block_Release(p_block);
- picture_Release( p_pic );
return NULL;
}
=====================================
modules/codec/vpx.c
=====================================
@@ -501,7 +501,6 @@ static block_t *Encode(encoder_t *p_enc, picture_t *p_pict)
/* Create and initialize the vpx_image */
if (!vpx_img_wrap(&img, VPX_IMG_FMT_I420, i_w, i_h, p_pict->p[0].i_pitch, p_pict->p[0].p_pixels)) {
VPX_ERR(p_enc, ctx, "Failed to wrap image");
- picture_Release(p_pict);
return NULL;
}
@@ -518,7 +517,6 @@ static block_t *Encode(encoder_t *p_enc, picture_t *p_pict)
if (res != VPX_CODEC_OK) {
VPX_ERR(p_enc, ctx, "Failed to encode frame");
vpx_img_free(&img);
- picture_Release(p_pict);
return NULL;
}
@@ -581,7 +579,6 @@ static block_t *Encode(encoder_t *p_enc, picture_t *p_pict)
webp_write_header(p_header, i_vp8_data_size, i_webp_header_size);
vpx_img_free(&img);
- picture_Release(p_pict);
return p_out;
}
=====================================
modules/misc/medialibrary/Thumbnailer.cpp
=====================================
@@ -49,7 +49,8 @@ void Thumbnailer::onThumbnailComplete( void* data, picture_t* thumbnail )
{
vlc::threads::mutex_locker lock( ctx->thumbnailer->m_mutex );
ctx->done = true;
- ctx->thumbnail = thumbnail;
+ if (thumbnail != nullptr)
+ ctx->thumbnail = picture_Hold(thumbnail);
ctx->thumbnailer->m_currentContext = nullptr;
}
ctx->thumbnailer->m_cond.signal();
@@ -86,13 +87,8 @@ bool Thumbnailer::generate( const medialibrary::IMedia&, const std::string& mrl,
vlc_thumbnailer_DestroyRequest(m_thumbnailer.get(), ctx.request);
ctx.request = NULL;
- /* Both picture_Export and ThumbnailerCtx will release the thumbnail, so
- * ensure that only picture_Export does. */
- picture_t *thumbnail = nullptr;
- std::swap(thumbnail, ctx.thumbnail);
-
block_t* block;
- if ( picture_Export( VLC_OBJECT( m_ml ), &block, nullptr, thumbnail,
+ if ( picture_Export( VLC_OBJECT( m_ml ), &block, nullptr, ctx.thumbnail,
VLC_CODEC_JPEG, desiredWidth, desiredHeight, true ) != VLC_SUCCESS )
return false;
auto blockPtr = vlc::wrap_cptr( block, &block_Release );
=====================================
src/input/thumbnailer.c
=====================================
@@ -213,7 +213,8 @@ RunnableRun(void *userdata)
if (notify)
NotifyThumbnail(task, pic);
- else if (pic != NULL)
+
+ if (pic)
picture_Release(pic);
input_Stop(input);
=====================================
src/misc/image.c
=====================================
@@ -393,12 +393,12 @@ static block_t *ImageWrite( image_handler_t *p_image, picture_t *p_pic,
{
p_image->p_enc = CreateEncoder( p_image->p_parent,
p_fmt_in, p_fmt_out );
- if( !p_image->p_enc ) {
- picture_Release( p_pic );
- return NULL;
- }
+ if( !p_image->p_enc ) return NULL;
}
+ /* We'll release the picture at the end or during conversion. */
+ picture_Hold(p_pic);
+
/* Check if we need chroma conversion or resizing */
if( p_image->p_enc->fmt_in.video.i_chroma != p_fmt_in->i_chroma ||
p_image->p_enc->fmt_in.video.i_width != p_fmt_in->i_width ||
@@ -430,7 +430,7 @@ static block_t *ImageWrite( image_handler_t *p_image, picture_t *p_pic,
if( !p_image->p_converter )
{
- picture_Release( p_pic );
+ picture_Release(p_pic);
return NULL;
}
}
@@ -443,17 +443,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 );
}
+ /* Hold the picture there to let the caller release its own picture,
+ * since filters will consume the picture. */
p_pic = p_image->p_converter->ops->filter_video( p_image->p_converter, p_pic );
-
- if( likely(p_pic != NULL) )
- {
- assert(!picture_HasChainedPics(p_pic)); // no chaining
- }
+ assert(p_pic == NULL || !picture_HasChainedPics(p_pic)); // no chaining
}
block_t *p_block = NULL;
if (p_pic != NULL)
+ {
p_block = vlc_encoder_EncodeVideo(p_image->p_enc, p_pic);
+ picture_Release(p_pic);
+ }
if( !p_block )
{
=====================================
test/src/input/thumbnail.c
=====================================
@@ -96,7 +96,6 @@ static void thumbnailer_callback( void* data, picture_t* thumbnail )
}
assert( thumbnail->date == expected_date && "Unexpected picture date");
#endif
- picture_Release( thumbnail );
}
else
assert( !test_params[p_ctx->test_idx].b_expected_success &&
=====================================
test/src/misc/image.c
=====================================
@@ -44,6 +44,8 @@ const char vlc_module_name[] = MODULE_STRING;
#include <limits.h>
+static atomic_bool encoder_opened = false;
+
static int OpenIntf(vlc_object_t *root)
{
image_handler_t *ih = image_HandlerCreate(root);
@@ -67,14 +69,19 @@ static int OpenIntf(vlc_object_t *root)
block = image_Write(ih, picture, &fmt_in, &fmt_out);
assert(block != NULL);
block_Release(block);
+ picture_Release(picture);
+ assert(atomic_load(&encoder_opened));
+ atomic_store(&encoder_opened, false);
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);
+ picture_Release(picture);
+ assert(atomic_load(&encoder_opened));
+
image_HandlerDelete(ih);
return VLC_SUCCESS;
@@ -82,10 +89,9 @@ static int OpenIntf(vlc_object_t *root)
static block_t * EncodeVideo(encoder_t *encoder, picture_t *pic)
{
- (void)encoder;
+ (void)encoder; (void)pic;
/* Dummy encoder */
- picture_Release(pic);
return block_Alloc(1);
}
@@ -97,6 +103,7 @@ static int OpenEncoder(vlc_object_t *obj)
.encode_video = EncodeVideo
};
encoder->ops = &ops;
+ atomic_store(&encoder_opened, true);
return VLC_SUCCESS;
}
=====================================
test/src/misc/image_cvpx.c
=====================================
@@ -85,6 +85,8 @@ static int OpenIntf(vlc_object_t *root)
block_Release(block);
image_HandlerDelete(ih);
+ picture_Release(picture);
+
return VLC_SUCCESS;
}
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/28bf3ccc4ded16a6a4c5e6bc21fabf2b61787bf7...46ca013bb6c23922609e1d936fc9f31cdcf7859f
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/28bf3ccc4ded16a6a4c5e6bc21fabf2b61787bf7...46ca013bb6c23922609e1d936fc9f31cdcf7859f
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