[Helix-client-dev] CR: Fixed the build depedency (bif) and streamtype mismatch for avi fileformat plugin
Zhao, Halley halley.zhao at intel.com¡¡¡¡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 ¡¡¡¡>> ¡¡¡¡>> ¡¡¡¡>> ¡¡¡¡>> -------------- next part -------------- A non-text attachment was scrubbed... Name: datatype-avi.diff Type: application/octet-stream Size: 3135 bytes Desc: datatype-avi.diff Url : http://lists.helixcommunity.org/pipermail/helix-client-dev/attachments/20070914/691afe62/datatype-avi-0001.obj