These changes look good to me. They are OK for
checkin to CVS.

Remember, since you are creating a new top-level
directory (mdf) in the /cvsroot/datatype repository, then
you can use "cvs import" rather than "cvs add" to
get those files in. However, I would caution that if
you have any binary files to check in, I would not
use "cvs import" to get those in, but rather add
them afterwards with "cvs add -kb".

Eric

> -----Original Message-----
> From: john.wei at nokia.com [mailto:john.wei at nokia.com] 
> Sent: Tuesday, October 04, 2005 9:39 AM
> To: ehyche at real.com; clientapps-dev at helixcommunity.org; 
> datatype-dev at helixcommunity.org; Ribosome-dev at helixcommunity.org
> Cc: john.wei at nokia.com
> Subject: RE: [datatype-dev] MDF Video Renderer - Part 1,2,3
> 
> 
> Hi,
> 
> I have incoporated all comments and enclosed here a copy of 
> all changed files.
> Please review and comment. If no comments submitted, I am 
> ready to check into CVS after 24 hours.
> Thank you for all of your time and efforts.
> 
> List of the changes:
> 
> changes related to CR part 1:
> 
> 1) helix.bif
>     <module id="datatype_mdf" name="datatype/mdf" group="core">
>    changes to:
>     <module id="datatype_mdf" name="datatype/mdf" 
> group="core" type="name_only">
> 
> 2) datatype/group/video/Umakefil:
> 
>    if "HELIX_FEATURE_SYMBIAN_MDF" in project.defines:
>    changes to:
>    if project.IsDefined("HELIX_FEATURE_SYMBIAN_MDF"):
> 
> 3) datatype/mp4/payload/Umakefil:
> 
>    if "HELIX_FEATURE_SYMBIAN_MDF" in project.defines:
>    changes to:
>    if project.IsDefined("HELIX_FEATURE_SYMBIAN_MDF"):
> 
> 4) helix-client-s60-basic.pf
> 
>    Add
>    project.AddDefines('HELIX_FEATURE_SYMBIAN_MDF')
> 
> changes related to CR part 2:
> 
> 1) In mdfvidrend.h, mdfvidren.cpp:
> 
>    HELIX_FEATURE_VIDREND_MDF_VELOCITY
>    changes tO;
>    HELIX_FEATURE_PLAYBACK_VELOCITY
> 
> 2) mdfvidrend.cpp and all other files
>    malloc/free/realloc
>    changes to
>    new/delete, realloc is removed
> 
> 3) mdfvidrend.cpp and all other files
>         HX_RELEASE( xxxx );
>         xxxx= NULL;
>    change to:
>         HX_RELEASE( xxxx );
> 
> 
> 4) all files
>    !SUCCEEDED( retVal )
>    changes to:
>    FAILED(retVal)
> 
> 5) For safety's sake, you may want to check that objects
>    are non-NULL before you use them. For example in OnPreSeek() you
>    use m_pMdfPayloadFormat without first checking that it's non-NULL.
>    Although it would be unlikely that anything past OnHeader() would
>    be called if OnHeader() failed, it's good practice to either check
>    or assert, and it's best practice to check AND assert.
> 
> DONE for all files as much possible. In some cases, no changes made
> when this NULL check is obviously unnecessaily
> 
> 6) mdfpayloadfomrat.h .cpp
> 
> introduce a new function
>     static void EndianChange( TInt8* aLocation, TInt aLength );
> 
> 
> 7) mdfvidrend.cpp
>     ulFlags = HX_DISPLAY_WINDOW |
>         HX_DISPLAY_SUPPORTS_RESIZE |
>         HX_DISPLAY_SUPPORTS_FULLSCREEN |
>         HX_DISPLAY_SUPPORTS_VIDCONTROLS;
> 
> changes to:
>     ulFlags = HX_DISPLAY_WINDOW |
>         HX_DISPLAY_SUPPORTS_RESIZE;
> 
> 8) permanent UIDs are used
> 
> 
> changes related to CR part 3:
> None
> 
> 
> br,
> 
> --john
> 
> -----Original Message-----
> From: ext Eric Hyche [mailto:ehyche at real.com]
> Sent: Friday, September 30, 2005 3:04 PM
> To: Wei John (Nokia-ES-MSW/Dallas); clientapps-dev at helixcommunity.org;
> datatype-dev at helixcommunity.org; Ribosome-dev at helixcommunity.org
> Subject: RE: [datatype-dev] MDF Video Renderer - Part2: MDF Video
> Renderer
> 
> 
>  
> John,
> 
> One comment below...
> 
> > -----Original Message-----
> > From: john.wei at nokia.com [mailto:john.wei at nokia.com] 
> > Sent: Friday, September 30, 2005 3:55 PM
> > To: ehyche at real.com; clientapps-dev at helixcommunity.org; 
> > datatype-dev at helixcommunity.org; Ribosome-dev at helixcommunity.org
> > Subject: RE: [datatype-dev] MDF Video Renderer - Part2: MDF 
> > Video Renderer
> > 
> > 
> > Hi, Eric,
> > 
> > Thank you for the comments. Please see below for my replies. 
> > I will travel to India next week. So there might be some 
> > delay in responding to the review.
> 
> <snip>
> 
> > 2) In CMdfVideoRenderer you are using
> >    malloc and free to allocate/free zm_pStreamMimeTypes.
> >    Why malloc/free versus new/delete?
> > 
> > ====================
> > JW: In mdfpluginmanager.cpp, I use realloc to form the 
> > pMimeType. Everytime CMdfPluginManager finds a set of 
> > decoder, pp and depacktizer that support a mimetype, 
> > pMimeType will be realloc to include this mimetype. 
> > new/delete does not have correponding method to "realloc". 
> > pMimeType is passed out of CMdfPluginManager by method:
> > 
> > zm_pStreamMimeTypes = m_pPluginManager->GetSupportedMimeType();
> > 
> > Since pMimeType is using malloc/free/realloc, I use 
> > malloc/free to deal with zm_pStreamMimeTypes, though 
> > new/delete is also OK in the case of zm_pStreamMimeTypes.
> > 
> > I have no opinion on this. Please tell me what you like. I 
> > could use whatever you feel comfatable.
> > ====================
> 
> Let's stick with new/delete if possible.
> 
> Eric
> 
> ==============================================
> Eric Hyche (ehyche at real.com)
> Embedded Player and Technologies
> RealNetworks, Inc. 
> 
> 
>