Looks good.

=============================================
Eric Hyche (ehyche at real.com)
Technical Lead
RealNetworks, Inc.  

> -----Original Message-----
> From: Zhao, Halley [mailto:halley.zhao at intel.com] 
> Sent: Thursday, September 13, 2007 10:50 PM
> To: ehyche at real.com; datatype-dev
> Cc: helix-client-dev at helixcommunity.org
> Subject: RE: [Helix-client-dev] CR: Fixed the build depedency 
> (bif) and streamtype mismatch for avi fileformat plugin
> 
> 　　Re-send diff file with acceptation of Eric's correction.
> 　　
> 　　It needs "#if defined(HELIX_FEATURE_SERVER)" to pass 
> compile for JPEGPayloadFormat.
> 　　
> 　　Pass Test.
> 　　
> 　　
> 　　>-----Original Message-----
> 　　>From: Eric Hyche [mailto:ehyche at real.com]
> 　　>Sent: 2007年9月13日 21:29
> 　　>To: Zhao, Halley; 'datatype-dev'
> 　　>Cc: helix-client-dev at helixcommunity.org
> 　　>Subject: RE: [Helix-client-dev] CR: Fixed the build 
> depedency (bif) and
> 　　>streamtype mismatch for avi fileformat plugin
> 　　>
> 　　>
> 　　>My comments on these changes:
> 　　>
> 　　> if project.IsDefined("HELIX_FEATURE_SERVER"):
> 　　>-    MultiTargetMake("aviffdll")
> 　　>+    MultiTargetMake("avifflib","aviffdll")
> 　　>
> 　　>No need to change if HELIX_FEATURE_SERVER is defined -
> 　　>this does not affect the client, and we should not
> 　　>change the server builds.
> 　　>
> 　　>-#if 0 // JPEGPayloadFormat not supported (yet)
> 　　>+// #if 0 // JPEGPayloadFormat not supported (yet)
> 　　>
> 　　>No need to keep this - just remove it.
> 　　>
> 　　>                         if (!m_bLocalPlayback)
> 　　>                         {
> 　　>+                            // ASSERT(0);
> 　　>+#if 0 // JPEGPayloadFormat not supported (yet)
> 　　>
> 　　>                             m_pPayloadFormatter = new
> 　　>JPEGPayloadFormat();
> 　　>
> 　　>This should not be ifdef'd out. if (!m_bLocalPlayback)
> 　　>means that this will only get executed when running on
> 　　>the server, so it should never get executed on the client.
> 　　>If there is a compilation issue with JPEGPayloadFormat,
> 　　>then the whole if (!m_bLocalPlayback) clause should be
> 　　>put inside #if defined(HELIX_FEATURE_SERVER).
> 　　>
> 　　>-#endif // 0
> 　　>+//#endif // 0
> 　　>
> 　　>Again, no reason to keep this.
> 　　>
> 　　>
> 　　>Rest looks good.
> 　　>
> 　　>Eric
> 　　>
> 　　>=============================================
> 　　>Eric Hyche (ehyche at real.com)
> 　　>Technical Lead
> 　　>RealNetworks, Inc.
> 　　>
> 　　>> -----Original Message-----
> 　　>> From: helix-client-dev-bounces at helixcommunity.org
> 　　>> [mailto:helix-client-dev-bounces at helixcommunity.org] On
> 　　>> Behalf Of Zhao, Halley
> 　　>> Sent: Thursday, September 13, 2007 3:06 AM
> 　　>> To: datatype-dev
> 　　>> Cc: helix-client-dev at helixcommunity.org
> 　　>> Subject: [Helix-client-dev] CR: Fixed the build depedency
> 　　>> (bif) and streamtype mismatch for avi fileformat plugin
> 　　>>
> 　　>> Bug Number:
> 　　>>
> 　　>>     7204
> 　　>>
> 　　>> Bug URL:
> 　　>>
> 　　>>     https://bugs.helixcommunity.org/show_bug.cgi?id=7204
> 　　>>
> 　　>> Synopsis:
> 　　>>
> 　　>>     fix a bug for endian type mismatch of avi file format module.
> 　　>>
> 　　>> Overview:
> 　　>>
> 　　>>     1.  Add  module dependlist in hxclient_3_1_0_atlas.bif
> 　　>> for avi fileformat target.
> 　　>>
> 　　>>     2.  avifformat.so plugin can't parse a valid avi stream ;
> 　　>> some confliction for little endian or big endian.
> 　　>>
> 　　>>     For 4-characters stream type string, such as "AVI
> 　　>> ","MJPEG","H263", helix transform it to an integer value,.
> 　　>> however sometimes it check the endian type of platform,
> 　　>> sometimes it use MACRO HX_MAKE4CC regardless endian type of
> 　　>> platform. it will cause mismatch.
> 　　>>
> 　　>>         a.  in aviffpln.cpp, there is check of the media stream:
> 　　>>
> 　　>>             if (m_pGeneralReader->FileSubtype() !=
> 　　>> HX_MAKE4CC('A', 'V', 'I', ' '))
> 　　>>
> 　　>>             FileSubtype defined in common module, it uses
> 　　>> platform dependent endian type, but HX_Make4CC use fixed
> 　　>> little endian. I think it's better to change to:
> 　　>>
> 　　>>             if (m_pGeneralReader->FileSubtype() !=
> 　　>> m_pGeneralReader->GetLong("AVI "))
> 　　>>
> 　　>>             FileSubtype() and GetLong() is both defined in
> 　　>> common module, and have the same endian type.
> 　　>>
> 　　>>         b.  avistrm.cpp
> 　　>>
> 　　>>             m_header.ulType                 =
> 　　>> LE32_TO_HOST(*(UINT32*) &buf[0])
> 　　>>
> 　　>>             LE32_TO_HOST depend on platfrom's endian type,
> 　　>> AVI_VIDS_TYPE, AVI_AUDS_TYPE, AVI_MJPG_VIDEO are defined by
> 　　>> HX_MAKE4CC regardless of endian type. there will be mismatch
> 　　>> when compare them.
> 　　>>
> 　　>>             the absolute value of ulType is not important, we
> 　　>> can modify as following to make them be consistent with 
> each other.
> 　　>>
> 　　>>             m_header.ulType = HX_MAKE4CC( buf[0], buf[1],
> 　　>> buf[2], buf[3])
> 　　>>
> 　　>>         c). mjpeg is supported by helix community now
> 　　>>
> 　　>>             Change the #if defined to enable mjpeg support.
> 　　>>
> 　　>>
> 　　>>
> 　　>> Files Added:
> 　　>>
> 　　>>        No file added
> 　　>>
> 　　>> Files Modified:
> 　　>>
> 　　>>     aviffpln.cpp: (datatype/avi/fileformat/aviffpln.cpp)
> 　　>>
> 　　>>         when compare with FileSubtype which defined in common
> 　　>> module, use GetLong (which also defined in common module) to
> 　　>> transform media type string to integer.
> 　　>>
> 　　>>     avistrm:    (datatype/avi/fileformat/avistrm.cpp)
> 　　>>
> 　　>>         use HX_MAKE4CC() to get ulType from media type
> 　　>> string, since it used inside avistrm.cpp only, and
> 　　>> AVI_VIDS_TYPE, AVI_AUDS_TYPE, AVI_MJPG_VIDEO are defined by
> 　　>> HX_MAKE4CC().
> 　　>>
> 　　>>
> 　　>>
> 　　>>         another solution is to redefine AVI_VIDS_TYPE,
> 　　>> AVI_AUDS_TYPE, AVI_MJPG_VIDEO by LE32_TO_HOST.
> 　　>>
> 　　>>
> 　　>>
> 　　>> 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:
> 　　>>
> 　　>>     avifformat.so
> 　　>>
> 　　>> Distribution library impact and planned action:
> 　　>>
> 　　>>     None
> 　　>>
> 　　>> Platforms and Profiles Build Verified:
> 　　>>
> 　　>>     Set BIF branch  -> hxclient_3_1_0_atlas_restricted
> 　　>>
> 　　>>     Set Target(s)   -> datatype_avi_fileformat
> 　　>>
> 　　>>     Set Profile     -> helix-client-all-defines
> 　　>>
> 　　>>     System ID       -> linux-2.2-libc6-gcc32-i586
> 　　>>
> 　　>> Branch:
> 　　>>
> 　　>>     HEAD, hxclient_3_1_0_atlas
> 　　>>
> 　　>> Copyright assignment: <MUST be one of the following statements >
> 　　>>
> 　　>>    3.      My company (company name here) submits this code
> 　　>> under the terms
> 　　>>
> 　　>>            of a commercial contribution agreement with 
> RealNetworks,
> 　　>>
> 　　>>            and I am authorized to contribute this code under
> 　　>> said agreement.
> 　　>>
> 　　>>
> 　　>>
> 　　>> Files Attached:
> 　　>>
> 　　>>     datatype-avi.diff
> 　　>>
> 　　>>     bif-avi.diff
> 　　>>
> 　　>>
> 　　>>
> 　　>> ZHAO, Halley (Aihua)
> 　　>>
> 　　>> Email: halley.zhao at intel.com <mailto:aihua.zhao at intel.com>
> 　　>>
> 　　>> Tel: +86(21)61166476
> 　　>>
> 　　>> iNet: 8821-6476
> 　　>>
> 　　>> SSG/OTC/UMD
> 　　>>
> 　　>>
> 　　>>
> 　　>>
>