There was a little bug in patch 4 - the pthread lock and condition could have ended up being not destroyed on provider->Release() - fixed now.
On Thu, 2010-05-20 at 17:05 +0100, Andre Draszik wrote: > Hi, > > I reworked my backgrounded image providers from last year, and it's all > much nicer now, less code duplication etc. > > Basically we now have a base struct + base code for image providers to > use, removing some common code (patches 1 and 2) in > src/media/idirectfbimageprovider.c > > Patch 3 adds the new API, plus a stub implementation to the base stuff. > > Patch 4 adds all the base support that is necessary for backgrounded > operation. > > Patches 5 and 6 switch the PNG and JPEG providers to using this stuff, > making them capable of background decoding, and again removing some > duplicated code. > > > This is all backwards compatible, i.e. existing image providers (e.g > DirectFB-extra) do not have to be modified, but they could, to profit > from common code elimination and backgrounded support, of course. > > I am also attaching a simple little test app. > > > What do you think? I think it is very useful in today's environments, as > it makes it much easier to use e.g. all available CPU cores on a desktop > system, or dedicated decoders in an embedded system (we have up to > 5 :-), without every app having to manage threading themselves just for > that. So I really want to get that in... :-) > > > Cheers, > Andre' >
>From 65007a8afca3bde4edc960b1b791af8c306df67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <andre.dras...@st.com> Date: Thu, 20 May 2010 16:06:37 +0100 Subject: [PATCH 4/6] image providers: implement base support for backgrounded image decoding The base IDirectFBImageProvider_data has been enhanced with additional callback functions that need to be implemented by a specific image provider implementation. In addition, they must not override RenderTo() anymore. This again simplifies code and actually reduces overall code size if image provider implementations choose to support backgrounded operation. This is all backwards compatible, i.e. if a specific image provider implementation does not want to support backgrounded image decoding (or hasn't been ported yet), no changes to existing providers are necessary. --- src/media/idirectfbimageprovider.c | 290 +++++++++++++++++++++++++++++++++++- src/media/idirectfbimageprovider.h | 22 +++ 2 files changed, 307 insertions(+), 5 deletions(-) diff --git a/src/media/idirectfbimageprovider.c b/src/media/idirectfbimageprovider.c index 9af02f6..9a3693b 100644 --- a/src/media/idirectfbimageprovider.c +++ b/src/media/idirectfbimageprovider.c @@ -41,6 +41,77 @@ #include <media/idirectfbimageprovider.h> #include <media/idirectfbdatabuffer.h> +#include "display/idirectfbsurface.h" + + +static DFBResult +IDirectFBImageProvider_RenderTo( IDirectFBImageProvider *thiz, + IDirectFBSurface *destination, + const DFBRectangle *destination_rect ); + + +static void +render_cleanup( void *cleanup_data ) +{ + IDirectFBImageProvider *thiz = cleanup_data; + IDirectFBImageProvider_data *data; + + D_MAGIC_ASSERT( (IAny*)thiz, DirectInterface ); + data = (IDirectFBImageProvider_data *) thiz->priv; + D_ASSERT( data != NULL ); + + if (data->destination) { + data->destination->Release( data->destination ); + data->destination = NULL; + } + + /* in case we get terminated from outside, set the state to DFB_DEAD */ + data->thread_res = DFB_DEAD; + + pthread_mutex_unlock( &data->lock ); +} + +static void * +render_thread( DirectThread *thread, + void *driver_data ) +{ + IDirectFBImageProvider *thiz = driver_data; + IDirectFBImageProvider_data *data; + IDirectFBSurface_data *dst_data; + CoreSurface *dst_surface; + DFBSurfacePixelFormat format; + DFBResult res; + + D_MAGIC_ASSERT( (IAny*)thiz, DirectInterface ); + data = (IDirectFBImageProvider_data *) thiz->priv; + D_ASSERT( data != NULL ); + + pthread_mutex_lock( &data->lock ); + + pthread_cleanup_push( render_cleanup, thiz ); + + /* wait until RenderTo() is called */ + while (!data->destination) + pthread_cond_wait( &data->cond, &data->lock ); + + dst_data = (IDirectFBSurface_data*) data->destination->priv; + D_ASSERT( dst_data != NULL ); + + dst_surface = dst_data->surface; + D_ASSERT( dst_surface != NULL ); + + res = data->destination->GetPixelFormat( data->destination, &format ); + D_ASSERT( res == DFB_OK ); + + res = data->RenderTo2( thiz, dst_surface, format, &data->rect, &data->clip ); + + pthread_cleanup_pop( 1 ); + + /* in case we exit normally, apply the real return value */ + data->thread_res = res; + + return NULL; +} static DirectResult IDirectFBImageProvider_AddRef( IDirectFBImageProvider *thiz ) @@ -58,6 +129,18 @@ IDirectFBImageProvider_Release( IDirectFBImageProvider *thiz ) DIRECT_INTERFACE_GET_DATA( IDirectFBImageProvider ) if (--data->ref == 0) { + /* terminate the decoding thread, if necessary */ + if (data->thread) { + direct_thread_cancel( data->thread ); + direct_thread_join( data->thread ); + direct_thread_destroy( data->thread ); + } + /* release thread related state */ + if (thiz->RenderTo == IDirectFBImageProvider_RenderTo) { + pthread_mutex_destroy( &data->lock ); + pthread_cond_destroy( &data->cond ); + } + if (data->Destruct) data->Destruct( thiz ); @@ -100,7 +183,81 @@ IDirectFBImageProvider_RenderTo( IDirectFBImageProvider *thiz, IDirectFBSurface *destination, const DFBRectangle *destination_rect ) { - return DFB_UNIMPLEMENTED; + IDirectFBSurface_data *dst_data; + CoreSurface *dst_surface; + DFBResult res; + + DIRECT_INTERFACE_GET_DATA( IDirectFBImageProvider ) + + dst_data = (IDirectFBSurface_data *) destination->priv; + if (!dst_data) + return DFB_DEAD; + + dst_surface = dst_data->surface; + if (!dst_surface) + return DFB_DESTROYED; + + if (D_FLAGS_IS_SET (data->flags, DIPF_BACKGROUND_DECODE)) + pthread_mutex_lock( &data->lock ); + + if (destination_rect) { + if (destination_rect->w < 1 || destination_rect->h < 1) { + if (D_FLAGS_IS_SET (data->flags, DIPF_BACKGROUND_DECODE)) + pthread_mutex_unlock( &data->lock ); + return DFB_INVARG; + } + + data->rect = *destination_rect; + data->rect.x += dst_data->area.wanted.x; + data->rect.y += dst_data->area.wanted.y; + } + else { + data->rect = dst_data->area.wanted; + } + + dfb_region_from_rectangle( &data->clip, &dst_data->area.current ); + + if (!dfb_rectangle_region_intersects( &data->rect, &data->clip )) { + if (D_FLAGS_IS_SET (data->flags, DIPF_BACKGROUND_DECODE)) + pthread_mutex_unlock( &data->lock ); + return DFB_OK; + } + + if (!D_FLAGS_IS_SET (data->flags, DIPF_BACKGROUND_DECODE)) { + /* threaded operation was not requested */ + DFBSurfacePixelFormat format; + res = destination->GetPixelFormat( destination, &format ); + if (res == DFB_OK) + res = data->RenderTo2( thiz, dst_surface, format, + &data->rect, &data->clip ); + + return DFB_OK; + } + + D_ASSERT( data->destination == NULL ); + + if (!data->thread) { + /* for the case that somebody does a RenderTo() twice on us, we + have to create new thread, because the initial thread will have + finished already */ + + /* as long as we haven't even started yet, we are in INIT state */ + data->thread_res = DFB_INIT; + data->thread = direct_thread_create( DTT_DEFAULT, render_thread, + thiz, data->DecoderName ); + } + + destination->AddRef( destination ); + data->destination = destination; + + /* as long as we haven't finished decoding we are busy. Need to do this + here so that we can see in Sync() that RenderTo() has beed called. */ + data->thread_res = DFB_BUSY; + + pthread_cond_signal( &data->cond ); + pthread_mutex_unlock( &data->lock ); + + return DFB_OK; } static DFBResult @@ -125,6 +282,103 @@ IDirectFBImageProvider_WriteBack( IDirectFBImageProvider *thiz, return DFB_UNIMPLEMENTED; } +static DFBResult +IDirectFBImageProvider_SetFlags( IDirectFBImageProvider *thiz, + DFBImageProviderFlags flags ) +{ + DIRECT_INTERFACE_GET_DATA( IDirectFBImageProvider ) + + /* if we have decoded the image already, don't do anything... */ + if (data->SetFlagsPrecheck) { + DFBResult res = data->SetFlagsPrecheck( thiz, flags ); + if (res != DFB_OK) + return res; + } + + /* modify state only if thread isn't running at the moment */ + if (data->thread + && data->thread_res != DFB_INIT) + return DFB_BUSY; + + if (!D_FLAGS_IS_SET( flags, DIPF_BACKGROUND_DECODE ) + && data->thread) { + /* terminate the decoding thread if necessary... */ + direct_thread_cancel( data->thread ); + direct_thread_join( data->thread ); + direct_thread_destroy( data->thread ); + data->thread = NULL; + + data->flags = flags & DIPF_BACKGROUND_DECODE; + } + else if (D_FLAGS_IS_SET( flags, DIPF_BACKGROUND_DECODE ) + && !data->thread) { + data->flags = flags & DIPF_BACKGROUND_DECODE; + + /* as long as we haven't even started yet, we are in INIT state */ + data->thread_res = DFB_INIT; + data->thread = direct_thread_create( DTT_DEFAULT, render_thread, + thiz, data->DecoderName ); + } + + return DFB_OK; +} + +static DFBResult +IDirectFBImageProvider_Sync( IDirectFBImageProvider *thiz, + DFBImageProviderSyncFlags flags ) +{ + DIRECT_INTERFACE_GET_DATA( IDirectFBImageProvider ) + + if (!D_FLAGS_IS_SET( data->flags, DIPF_BACKGROUND_DECODE )) + return DFB_OK; + + if (data->thread + && data->thread_res == DFB_INIT) + /* calling Sync() while we are in init state basically means + RenderTo() has not been called yet, i.e. we have no destination + surface. This is not a good idea, as the thread will be waiting + for RenderTo() to be called, thus the join below would sleep + forever. + We could potentially kill the thread here instead, but if killing + it is the intention, then SetFlags() should be used instead in + the first place. */ + return DFB_NOCONTEXT; + + switch (flags) + { + case DIPSF_TRYSYNC: + if (data->thread) { + if (data->thread_res == DFB_BUSY) + /* still busy decoding */ + return DFB_BUSY; + /* else we are done, either because of some error or because + we have processed all the data already */ + } + /* fall through */ + + case DIPSF_SYNC: + if (data->thread) { + direct_thread_join( data->thread ); + direct_thread_destroy( data->thread ); + data->thread = NULL; + } + break; + + default: + return DFB_INVARG; + } + + return DFB_OK; +} + +static DFBResult +IDirectFBImageProvider_GetResult( IDirectFBImageProvider *thiz ) +{ + DIRECT_INTERFACE_GET_DATA( IDirectFBImageProvider ) + + return data->thread_res; +} + /* non backgrounded versions */ static DFBResult IDirectFBImageProvider_SetFlags_nb( IDirectFBImageProvider *thiz, @@ -170,9 +424,9 @@ IDirectFBImageProvider_Construct( IDirectFBImageProvider *thiz ) thiz->RenderTo = IDirectFBImageProvider_RenderTo; thiz->SetRenderCallback = IDirectFBImageProvider_SetRenderCallback; thiz->WriteBack = IDirectFBImageProvider_WriteBack; - thiz->SetFlags = IDirectFBImageProvider_SetFlags_nb; - thiz->Sync = IDirectFBImageProvider_Sync_nb; - thiz->GetResult = IDirectFBImageProvider_GetResult_nb; + thiz->SetFlags = IDirectFBImageProvider_SetFlags; + thiz->Sync = IDirectFBImageProvider_Sync; + thiz->GetResult = IDirectFBImageProvider_GetResult; } DFBResult @@ -220,8 +474,34 @@ IDirectFBImageProvider_CreateFromBuffer( IDirectFBDataBuffer *buffer, if (ret) return ret; + /* some consistency checks */ + if (imageprovider->RenderTo == IDirectFBImageProvider_RenderTo) { + IDirectFBImageProvider_data *data = + (IDirectFBImageProvider_data *) imageprovider->priv; + + D_ASSERT( data != NULL ); + + D_ASSERT( data->DecoderName != NULL ); + D_ASSERT( data->RenderTo2 != NULL ); + + /* create thread state */ + pthread_cond_init( &data->cond, NULL ); + pthread_mutex_init( &data->lock, NULL ); + } + else { + /* if the image provider has its own version of RenderTo(), then + it doesn't support backgrounded operation, and it might not + (yet) have been converted to use the new + IDirectFBImageProvider_data base struct. */ + if (imageprovider->SetFlags == IDirectFBImageProvider_SetFlags) + imageprovider->SetFlags = IDirectFBImageProvider_SetFlags_nb; + if (imageprovider->Sync == IDirectFBImageProvider_Sync) + imageprovider->Sync = IDirectFBImageProvider_Sync_nb; + if (imageprovider->GetResult == IDirectFBImageProvider_GetResult) + imageprovider->GetResult = IDirectFBImageProvider_GetResult_nb; + } + *interface = imageprovider; return DFB_OK; } - diff --git a/src/media/idirectfbimageprovider.h b/src/media/idirectfbimageprovider.h index cbc81ce..3e84dbf 100644 --- a/src/media/idirectfbimageprovider.h +++ b/src/media/idirectfbimageprovider.h @@ -62,6 +62,28 @@ typedef struct { void *render_callback_context; void (*Destruct)( IDirectFBImageProvider *thiz ); + + /* background decoding */ + DFBImageProviderFlags flags; + pthread_mutex_t lock; + pthread_cond_t cond; + DirectThread *thread; + IDirectFBSurface *destination; + DFBResult thread_res; + + DFBRectangle rect; + DFBRegion clip; + + /* these will need to be implemented, if background decoding support + is wanted by a specific implementation. */ + const char *DecoderName; + DFBResult (*SetFlagsPrecheck)( IDirectFBImageProvider *thiz, + DFBImageProviderFlags flags ); + DFBResult (*RenderTo2)( IDirectFBImageProvider *thiz, + CoreSurface *dst_surface, + DFBSurfacePixelFormat format, + DFBRectangle *rect, + const DFBRegion *clip ); } IDirectFBImageProvider_data; -- 1.7.0.4
_______________________________________________ directfb-dev mailing list directfb-dev@directfb.org http://mail.directfb.org/cgi-bin/mailman/listinfo/directfb-dev