RE: [Re-Send] Add power save mode support for offload music playback (Framework part only)

Thanks Sheldon:
I will follow it up.

I test it by adding some hard code in engine/mp3-codec. That's really rough.


-----Original Message-----
From: Sheldon fu [mailto:[email protected]] 
Sent: Tuesday, September 01, 2009 4:42 AM
To: Zhao, Halley
Cc: '[email protected]'
Subject: Re: [Helix-client-dev] [Re-Send] Add power save mode support for offload music playback (Framework part only)

1. pIPowerSave needs to be released in the new methods added to
CHXClientEngine.cpp.

2. Can not return HXR_FAIL from methods with HXBOOL (or anything other
than HX_RESULT) return type, in file CHXClientEngine.cpp.

3. STDMETHODIMP_ shouldn't be used in the header file CHXClientEngine.h.
Should just use STDMETHOD or STDMETHOD_

4. In hxcore.h, "STDMETHOD_(HX_RESULT,xxx)" should be just
"STDMETHOD(xxx)".

5. In hxcleng.cpp

+        LISTPOSITION lPosition = m_PlayerList.GetHeadPosition();
+        if (lPosition != NULL)
+        {
+            HXPlayer* pPlayer = (HXPlayer*)
m_PlayerList.GetAt(lPosition);
+            if (pPlayer != NULL)
+                pPlayer->AdjustWakeUpInterval(m_ulWakeUpInterval);

can be simplified by using m_PlayerList.GetHead or
m_PlayerList.GetTail().

6. Helix coding convention normally suggests putting body of conditional
statements in '{}' pair, at least in the client core source files. e.g, 

+    if (theErr == HXR_OK)
+        m_bInPowerSave = TRUE;
+    else
+        EndPowerSave();

would be

+    if (theErr == HXR_OK)
+    {
+        m_bInPowerSave = TRUE;
+    }
+    else
+    {
+        EndPowerSave();
+    }

7. in hxplay.cpp

+            if (((HXFileSource*)pSource)->UsesNetworkAccess())

is dangerous. You may be casting a HXNetSource to HXFileSource.

The overall logic looks ok to me.

I assume this is un-tested code since we don't have any audio renderer
that supports IHXPowerSave interface and behavior yet, right?

fxd

On Fri, 2009-08-28 at 10:21 +0800, Zhao, Halley wrote:
> Send it again since there is no response yet.
> 
>  
> 
>                                    
> ______________________________________________________________________
> From:[email protected]
> [mailto:[email protected]] On Behalf Of
> Zhao, Halley
> Sent: 2009年8月20日15:09
> To: '[email protected]'
> Subject: [Helix-client-dev] Add power save mode support for offload
> music playback (Framework part only)
> 
> 
>  
> 
> Synopsis: 
> 
> Some audio dsp could playback(decode and render) long period of audio
> data without interaction with Helix core.
> 
> we need reduce wakeup from Helix audio service to achieve power
> saving.
> 
> This is achieved by:
> 
>     audio codec/render doesn't register audio stream to audio service
> idle.
> 
>     audio codec/render register as a IHXClockSource to drive the
> timeline
> 
>                                
> 
> Overview: 
> 
>     Details see attached design-note.txt
> 
>          
> 
> Files Added:
> 
>        No file added
> 
> Files Modified:
> 
> |-- client
> 
> |   |-- audiosvc
> 
> |   |   |-- hxaudply.cpp
> 
> |   |   |-- hxaudses.cpp
> 
> |   |   `-- pub
> 
> |   |       |-- hxaudply.h
> 
> |   |       `-- hxaudses.h
> 
> |   `-- core
> 
> |       |-- hxbsrc.h
> 
> |       |-- hxcleng.cpp
> 
> |       |-- hxflsrc.cpp
> 
> |       |-- hxflsrc.h
> 
> |       |-- hxplay.cpp
> 
> |       `-- pub
> 
> |           |-- hxcleng.h
> 
> |           `-- hxplay.h
> 
> |-- common
> 
> |   `-- include
> 
> |       |-- hxausvc.h
> 
> |       |-- hxcore.h
> 
> |       `-- hxevent.h
> 
> |-- list.txt
> 
> `-- player
> 
>     `-- hxclientkit
> 
>         |-- pub
> 
>         |   `-- HXClientCFuncs.h
> 
>         `-- src
> 
>             |-- CHXClientEngine.cpp
> 
>             |-- CHXClientEngine.h
> 
>             `-- HXClientCFuncs.cpp
> 
>  
> 
> Image Size and Heap Use impact (Client -Only):
> 
>     little
> 
> Platforms and Profiles Affected:
> 
>     platform:   linux-2.2-libc6-gcc32-i586
> 
>     profile:    helix-client-all-defines
> 
>  
> 
> Distribution Libraries Affected:
> 
>     <hxmedpcleng.so>
> 
> Distribution library impact and planned action:
> 
>     <None>
> 
> Platforms and Profiles Build Verified:
> 
>     Set BIF branch  -> realplay_gtk_atlas_restricted
> 
>     Set Target(s)   -> player_all
> 
>     Set Profile     -> helix-client-moblin
> 
>     System ID       -> linux-2.2-libc6-gcc32-i586
> 
> Branch: 
> 
>     HEAD
> 
> Copyright assignment: <MUST be one of the following statements >
> 
>    2.      Intel has signed and delivered a Joint Copyright Assignment
> 
>            to RealNetworks, and received acknowledgment that the 
> 
>            agreement was received. 
> 
> Files Attached:
> 
> Design-note.txt, client-audiosvc.diff.txt, client-core.diff.txt,
> common-include.diff.txt, player-hxclientkit.diff.txt
> 
>  
> 
>  
> 
> ZHAO, Halley (Aihua)
> 
> Email: [email protected]
> 
> Tel: +86(21)61166476
> 
> iNet: 8821-6476
> 
> SSG/OTC/Moblin 3W038 Pole: F4
> 
> 
>  
> 
> 
> _______________________________________________
> Helix-client-dev mailing list
> [email protected]
> http://lists.helixcommunity.org/mailman/listinfo/helix-client-dev

_______________________________________________
Helix-client-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/helix-client-dev