[prev in list] [next in list] [prev in thread] [next in thread] 

List:       helix-video-dev
Subject:    Re: [Video-dev] CR: Video changes for the Symbian MMF implementation
From:       Greg Wright <gwright () real ! com>
Date:       2005-09-29 18:28:57
Message-ID: 433C3269.8090407 () real ! com
[Download RAW message or body]

Darrin.Mann@nokia.com wrote:
> "NOKIA is bound by the terms of a commercial contribution agreement with \
> RealNetworks, and I am authorized to contribute this code under said agreement." 
> Modified by:  darrin.mann@nokia.com
> 
> Reviewed by:
> 
> Date:  09-28-2005.
> 
> Project:  Helix MMF Implementation for Symbian
> 
> Synopsis:  
> 
> This CR is part of the overall MMF implementation for the Helix engine.  This CR \
> modifies the sitelib portion of the video module to account for the lack of access \
> to the DSA for the MMF implementation. 
> This CR is dependent on CRs submitted for the MMF implementation to the clientapps, \
> common, filesystem, and ribosome modules. 
> 
> Files Modified:  video\site\symbian.pcf
> video\sitelib\pub\platform\symbian\minisymbiansurf.h
> video\sitelib\platform\symbian\minisymbiansite.cpp
> video\sitelib\platform\symbian\minisymbiansurf.cpp
> 
> 
> Files Added:  None
> 
> Image Size and Heap Use impact:  Minor
> 
> Platforms and Profiles Build Verified:  helix-client-s60-mmf-basic
> 
> Platforms and Profiles Functionality verified:  symbian-70s-thumb & \
> symbian-70s-emulator with  
> Branch:  Helix


 >Index: site/symbian.pcf
 >===================================================================
 >RCS file: /cvsroot/video/site/symbian.pcf,v
 >retrieving revision 1.2
 >diff -u -w -r1.2 symbian.pcf
 >--- site/symbian.pcf	9 Jul 2004 18:35:49 -0000	1.2
 >+++ site/symbian.pcf	28 Sep 2005 23:28:48 -0000
 >@@ -55,3 +55,6 @@
 > project.AddModuleLibraries("video/sitelib[sitelib]");
 >
 > project.AddSystemLibraries("ws32.lib", "bitgdi.lib", "gdi.lib", "fbscli.lib")
 >+
 >+if (project.IsDefined('HELIX_FEATURE_SYMBIAN_MMF')):
 >+    project.AddSystemLibraries("hal.lib")


 >Index: sitelib/platform/symbian/minisymbiansurf.cpp
 >===================================================================
 >RCS file: /cvsroot/video/sitelib/platform/symbian/minisymbiansurf.cpp,v
 >retrieving revision 1.26
 >diff -u -w -r1.26 minisymbiansurf.cpp
 >--- sitelib/platform/symbian/minisymbiansurf.cpp	21 Feb 2005 16:27:19 -0000	1.26
 >+++ sitelib/platform/symbian/minisymbiansurf.cpp	28 Sep 2005 23:28:48 -0000
 >@@ -47,6 +47,7 @@
 >  *
 >  * ***** END LICENSE BLOCK ***** */
 >
 >+#include "hal.h"


the HAL lib is not included unless HELIX_FEATURE_SYMBIAN_MMF is defined.
I would guess you would want to wrap the .h file as well.


 >@@ -66,6 +67,24 @@
 > #include <gdi.h>
 > #include <fbs.h>
 >
 >+
 >+static void GetDisplayMode(TDisplayMode &displayMode)
 >+{
 >+    TInt colors;
 >+    HAL::Get(HALData::EDisplayColors, colors);
 >+
 >+    switch (colors)
 >+    {
 >+    case 4096:
 >+        displayMode = EColor4K; break;
 >+    case 65536:
 >+        displayMode = EColor64K; break;
 >+    case 16777216:
 >+    default:
 >+        displayMode = EColor16MU;

You want the default to be 16 million colors?

 >+    }
 >+}
 >+


It looks like GetDisplayMode is only used if HELIX_FEATURE_SYMBIAN_MMF is
defined. If so, you should wrap it as well. Better yet, anyway we can use
that for all Symbian builds so that we don't have to hard code it for non
MMF builds?


 > CMiniSymbianSurface::CMiniSymbianSurface(IUnknown* pContext, CMiniBaseSite* pSite)
 >     :  CMiniBaseSurface(pContext, pSite),
 >        m_nCompositionPitch(0),
 >@@ -80,6 +99,9 @@
 >     memset(&m_surfaceSize, 0, sizeof(m_surfaceSize) );
 >     memset(&m_ColorData, 0, sizeof(m_ColorData) );
 >
 >+#if defined(HELIX_FEATURE_SYMBIAN_MMF)
 >+    m_pFbsScreenDevice = NULL;
 >+#endif
 > }
 >
 > CMiniSymbianSurface::~CMiniSymbianSurface()
 >@@ -91,6 +113,11 @@
 >     if( m_pBS )
 >         m_pBS->Disconnect();
 >     HX_DELETE(m_pBS);
 >+#if defined(HELIX_FEATURE_SYMBIAN_MMF)
 >+    HX_DELETE(m_pFbsScreenDevice);
 >+    delete m_Gc;

How about a HX_DELETE(m_Gc) so it gets NULL'ed out?

 >+    m_Ws.Close();
 >+#endif
 > }
 >
 > color_data_t* CMiniSymbianSurface::GetColorData()
 >@@ -111,7 +138,6 @@
 >     m_surfaceSize.cy = nHeight;
 >
 >     //Get our GC, and Region.
 >-    m_DisplayMode = EColor4K; //hardcoded for emulator right now.
 >
 >     HX_RESULT retVal = HXR_OK;
 >     if( m_pImageHelper )
 >@@ -222,7 +248,14 @@
 >     HX_RESULT res = HXR_OK;
 >
 >     HX_ASSERT(m_pBitMap != 0);
 >+
 >+    //
 >+    //  Must lock/unlock the heap for CodeWarrior
 >+    //
 >+    m_pBitMap->LockHeap();

What if you are not on CodeWarrior? Anything bad going
to happen?


 >     *ppDestPtr   = (UCHAR*)m_pBitMap->DataAddress();
 >+    m_pBitMap->UnlockHeap();
 >+

After the UnlockHeap, can we still trust ppDestPtr points
to valid memory?


 >     *pnDestPitch = m_nCompositionPitch;
 >     cid          = CID_RGB444;
 >
 >@@ -282,11 +315,47 @@
 >     m_pSite->GetPosition(point);
 > #endif
 >
 >+#if defined(HELIX_FEATURE_SYMBIAN_MMF)
 >+    if(m_pFbsScreenDevice == NULL)
 >+    {
 >+        m_Ws.Connect();
 >+        m_Gc = CFbsBitGc::NewL();
 >+
 >+        // Initilize the Screen Device needed for DSA
 >+
 >+        TDisplayMode displayMode;
 >+        GetDisplayMode(displayMode);
 >+        TRAPD(err, m_pFbsScreenDevice = CFbsScreenDevice::NewL(_L("scdv"), \
displayMode));  >+        m_Gc->Activate((CFbsDevice*)m_pFbsScreenDevice);
 >+
 >+        if(err != KErrNone)
 >+        {
 >+            return HXR_FAILED;
 >+        }
 >+    }
 >+    CFbsScreenDevice* pSD = m_pFbsScreenDevice;
 >+#else
 >     CFbsScreenDevice*& pSD = pDirectScreen->ScreenDevice();
 >+#endif
 >     if( pSD )
 >     {
 >+#if defined(HELIX_FEATURE_SYMBIAN_MMF)
 >+        if(!m_pGC)
 >+        {
 >+            pSD->CreateContext(m_pGC);
 >+        }
 >+
 >+        TRect clipRect( pHXWin->clipRect.left,
 >+                        pHXWin->clipRect.top,
 >+                        pHXWin->clipRect.right,
 >+                        pHXWin->clipRect.bottom );
 >+
 >+        RRegion* mmfClipReg = new RRegion(clipRect);
 >+        clipReg = mmfClipReg;
 >+#else
 >         m_pGC     = pDirectScreen->Gc();
 >         clipReg   = pDirectScreen->DrawingRegion();
 >+#endif
 >         if( clipReg )
 >         {
 >
 >@@ -378,6 +447,9 @@
 > #if defined(HELIX_FEATURE_SMIL_SITE)
 >             trTmp.Close();
 > #endif
 >+#if defined(HELIX_FEATURE_SYMBIAN_MMF)
 >+             delete mmfClipReg;

Perhaps HX_DELETE....

The rest looks good.
--greg.




 >+#endif
 >         }
 >     }
 >
 >Index: sitelib/pub/platform/symbian/minisymbiansurf.h
 >===================================================================
 >RCS file: /cvsroot/video/sitelib/pub/platform/symbian/minisymbiansurf.h,v
 >retrieving revision 1.16
 >diff -u -w -r1.16 minisymbiansurf.h
 >--- sitelib/pub/platform/symbian/minisymbiansurf.h	11 Mar 2005 19:58:09 -0000	1.16
 >+++ sitelib/pub/platform/symbian/minisymbiansurf.h	28 Sep 2005 23:28:48 -0000
 >@@ -105,6 +105,12 @@
 >     RFbsSession*         m_pBS;
 >     ULONG32              m_ulLastResetTime;
 >     HXBOOL                 m_bAutoRotate;
 >+
 >+#if defined(HELIX_FEATURE_SYMBIAN_MMF)
 >+    CFbsScreenDevice* m_pFbsScreenDevice;
 >+    CFbsBitGc*        m_Gc;
 >+    RWsSession        m_Ws;
 >+#endif
 > };
 >
 > #endif //_MSYMBIANSURF_H_


[prev in list] [next in list] [prev in thread] [next in thread]