From 30005be2e5eb30348f95d4af76f199c69f680f0c Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 5 Dec 2011 17:57:01 +0000 Subject: [PATCH] basevideo: Make GstVideoFrame a reference counted boxed object ...and also clear all existing frames when resetting the decoder or encoder. --- omx/gstbasevideocodec.c | 35 ++++++++++++++++++++++++++++++++--- omx/gstbasevideocodec.h | 7 ++++++- omx/gstbasevideodecoder.c | 23 ++++++++++++++--------- omx/gstbasevideoencoder.c | 13 ++++--------- 4 files changed, 56 insertions(+), 22 deletions(-) diff --git a/omx/gstbasevideocodec.c b/omx/gstbasevideocodec.c index 68e203d..45e2a5c 100644 --- a/omx/gstbasevideocodec.c +++ b/omx/gstbasevideocodec.c @@ -45,6 +45,9 @@ static void gst_base_video_codec_finalize (GObject * object); static GstStateChangeReturn gst_base_video_codec_change_state (GstElement * element, GstStateChange transition); +G_DEFINE_BOXED_TYPE (GstVideoState, gst_video_frame, + (GBoxedCopyFunc) gst_video_frame_ref, + (GBoxedFreeFunc) gst_video_frame_unref); GST_BOILERPLATE (GstBaseVideoCodec, gst_base_video_codec, GstElement, GST_TYPE_ELEMENT); @@ -109,7 +112,7 @@ gst_base_video_codec_reset (GstBaseVideoCodec * base_video_codec) GST_BASE_VIDEO_CODEC_STREAM_LOCK (base_video_codec); for (g = base_video_codec->frames; g; g = g_list_next (g)) { - gst_base_video_codec_free_frame ((GstVideoFrame *) g->data); + gst_video_frame_unref ((GstVideoFrame *) g->data); } g_list_free (base_video_codec->frames); base_video_codec->frames = NULL; @@ -175,16 +178,21 @@ gst_base_video_codec_new_frame (GstBaseVideoCodec * base_video_codec) frame = g_slice_new0 (GstVideoFrame); + frame->ref_count = 1; + GST_BASE_VIDEO_CODEC_STREAM_LOCK (base_video_codec); frame->system_frame_number = base_video_codec->system_frame_number; base_video_codec->system_frame_number++; GST_BASE_VIDEO_CODEC_STREAM_UNLOCK (base_video_codec); + GST_LOG_OBJECT (base_video_codec, "Created new frame %p (sfn:%d)", + frame, frame->system_frame_number); + return frame; } -void -gst_base_video_codec_free_frame (GstVideoFrame * frame) +static void +_gst_video_frame_free (GstVideoFrame * frame) { g_return_if_fail (frame != NULL); @@ -204,3 +212,24 @@ gst_base_video_codec_free_frame (GstVideoFrame * frame) g_slice_free (GstVideoFrame, frame); } + +GstVideoFrame * +gst_video_frame_ref (GstVideoFrame * frame) +{ + g_return_val_if_fail (frame != NULL, NULL); + + g_atomic_int_inc (&frame->ref_count); + + return frame; +} + +void +gst_video_frame_unref (GstVideoFrame * frame) +{ + g_return_if_fail (frame != NULL); + g_return_if_fail (frame->ref_count > 0); + + if (g_atomic_int_dec_and_test (&frame->ref_count)) { + _gst_video_frame_free (frame); + } +} diff --git a/omx/gstbasevideocodec.h b/omx/gstbasevideocodec.h index 074537a..41c2282 100644 --- a/omx/gstbasevideocodec.h +++ b/omx/gstbasevideocodec.h @@ -110,6 +110,8 @@ struct _GstVideoState struct _GstVideoFrame { + gint ref_count; + GstClockTime decode_timestamp; GstClockTime presentation_timestamp; GstClockTime presentation_duration; @@ -179,10 +181,13 @@ struct _GstBaseVideoCodecClass void *padding[GST_PADDING_LARGE]; }; +GType gst_video_frame_get_type (void); GType gst_base_video_codec_get_type (void); GstVideoFrame * gst_base_video_codec_new_frame (GstBaseVideoCodec *base_video_codec); -void gst_base_video_codec_free_frame (GstVideoFrame *frame); + +GstVideoFrame * gst_video_frame_ref (GstVideoFrame * frame); +void gst_video_frame_unref (GstVideoFrame * frame); G_END_DECLS diff --git a/omx/gstbasevideodecoder.c b/omx/gstbasevideodecoder.c index f396ca0..6a5554e 100644 --- a/omx/gstbasevideodecoder.c +++ b/omx/gstbasevideodecoder.c @@ -922,16 +922,19 @@ gst_base_video_decoder_clear_queues (GstBaseVideoDecoder * dec) g_list_foreach (dec->gather, (GFunc) gst_mini_object_unref, NULL); g_list_free (dec->gather); dec->gather = NULL; - g_list_foreach (dec->decode, (GFunc) gst_base_video_codec_free_frame, NULL); + g_list_foreach (dec->decode, (GFunc) gst_video_frame_unref, NULL); g_list_free (dec->decode); dec->decode = NULL; g_list_foreach (dec->parse, (GFunc) gst_mini_object_unref, NULL); g_list_free (dec->parse); dec->parse = NULL; - g_list_foreach (dec->parse_gather, (GFunc) gst_base_video_codec_free_frame, - NULL); + g_list_foreach (dec->parse_gather, (GFunc) gst_video_frame_unref, NULL); g_list_free (dec->parse_gather); dec->parse_gather = NULL; + g_list_foreach (GST_BASE_VIDEO_CODEC (dec)->frames, + (GFunc) gst_video_frame_unref, NULL); + g_list_free (GST_BASE_VIDEO_CODEC (dec)->frames); + GST_BASE_VIDEO_CODEC (dec)->frames = NULL; } static void @@ -965,7 +968,7 @@ gst_base_video_decoder_reset (GstBaseVideoDecoder * base_video_decoder, base_video_decoder->timestamps = NULL; if (base_video_decoder->current_frame) { - gst_base_video_codec_free_frame (base_video_decoder->current_frame); + gst_video_frame_unref (base_video_decoder->current_frame); base_video_decoder->current_frame = NULL; } @@ -1094,8 +1097,10 @@ gst_base_video_decoder_flush_decode (GstBaseVideoDecoder * dec) next = g_list_next (walk); if (dec->current_frame) - gst_base_video_codec_free_frame (dec->current_frame); + gst_video_frame_unref (dec->current_frame); dec->current_frame = frame; + gst_video_frame_ref (dec->current_frame); + /* decode buffer, resulting data prepended to queue */ res = gst_base_video_decoder_have_frame_2 (dec); @@ -1471,10 +1476,7 @@ gst_base_video_decoder_do_finish_frame (GstBaseVideoDecoder * dec, GST_BASE_VIDEO_CODEC (dec)->frames = g_list_remove (GST_BASE_VIDEO_CODEC (dec)->frames, frame); - if (frame->src_buffer) - gst_buffer_unref (frame->src_buffer); - - gst_base_video_codec_free_frame (frame); + gst_video_frame_unref (frame); } /** @@ -1861,6 +1863,9 @@ gst_base_video_decoder_have_frame_2 (GstBaseVideoDecoder * base_video_decoder) } exit: + /* current frame has either been added to parse_gather or sent to + handle frame so there is no need to unref it */ + /* create new frame */ base_video_decoder->current_frame = gst_base_video_decoder_new_frame (base_video_decoder); diff --git a/omx/gstbasevideoencoder.c b/omx/gstbasevideoencoder.c index c14d7a1..e4fe35f 100644 --- a/omx/gstbasevideoencoder.c +++ b/omx/gstbasevideoencoder.c @@ -292,14 +292,9 @@ gst_base_video_encoder_drain (GstBaseVideoEncoder * enc) /* everything should be away now */ if (codec->frames) { /* not fatal/impossible though if subclass/codec eats stuff */ - GST_WARNING_OBJECT (enc, "still %d frames left after draining", - g_list_length (codec->frames)); -#if 0 - /* FIXME should do this, but subclass may come up with it later on ? - * and would then need refcounting or so on frames */ - g_list_foreach (codec->frames, - (GFunc) gst_base_video_codec_free_frame, NULL); -#endif + g_list_foreach (codec->frames, (GFunc) gst_video_frame_unref, NULL); + g_list_free (codec->frames); + codec->frames = NULL; } return ret; @@ -1121,7 +1116,7 @@ done: GST_BASE_VIDEO_CODEC (base_video_encoder)->frames = g_list_remove (GST_BASE_VIDEO_CODEC (base_video_encoder)->frames, frame); - gst_base_video_codec_free_frame (frame); + gst_video_frame_unref (frame); GST_BASE_VIDEO_CODEC_STREAM_UNLOCK (base_video_encoder); -- 2.7.4