[datatype-dev] CR: Changes to Support REPEAT Tag in asxrender
Sushant Garg sgarg at real.comHi Eric, Please find the attached updated diff for details. Thanks & Regards Sushant Garg ----- Original Message ----- From: "Eric Hyche" <ehyche at real.com> To: "'Deepak Jain'" <deepakj at real.com> Cc: <datatype-dev at helixcommunity.org> Sent: Wednesday, December 17, 2008 11:30 PM Subject: RE: [datatype-dev] CR: Changes to Support REPEAT Tag in asxrender > Deepak, > > Here are my comments: > > + UINT32 m_RepeatCount; > > We should follow normal naming convention and > call this m_ulRepeatCount. > > +#define MAX_REPEAT_COUNT 0x05 //Max Number of counts for a <repeat> tag. > + > > This should certainly be bigger than 5. Maybe > this could be something like 100. > > + , m_RepeatCount(NULL) > > Pointer variables should be initialized to NULL. Integer > variables like this one should be initialized to 0. > > Your fix changes the semantics of GetNumChildren() by > making it recursive for <repeat> elements. I don't > think we should do that. GetNumChildren() and > GetIthChild() have very clear and simple semantics right > now, and I don't think we should change those semantics. > Also, you have a lot of repeated code that looks like > this: > > + if(m_bRepeat) > + { > + if(ulEntryIndex) > + { > + ulEntryIndex= ulEntryIndex%NumChildren; > + } > + } > > Right now we are computing the number of > entries (clips in the playlist) by doing this: > > UINT32 ulRet = GetNumChildren(m_pRoot, HXASXElementEntry | > HXASXElementEntryRef); > > which gives us the number of <entry> and <entryref> elements > which are the direct children of the <asx> element. > > So now, when we take <repeat> into account, what we want is > to ask: "how many <entry>, <entryref>, and <repeat> > elements are direct children of the <asx> element?" > Then, for <repeat> elements, we have to go one level down. > > So now we can compute the number of entries like this: > > UINT32 CASXRenderer:: GetNumEntriesWithRepeat() > { > UINT32 ulAllEntries = GetNumChildren(m_pRoot, > HXASXElementEntry | > HXASXElementEntryRef | HXASXElementRepeat); > UINT32 ulNumRepeats = GetNumChildren(m_pRoot, HXASXElementRepeat); > UINT32 ulRepeatEntries = 0; > if (ulNumRepeats) > { > UINT32 i = 0; > for (i = 0; i < ulAllEntries; i++) > { > CHXASXElement* pEntry = NULL; > retVal = GetIthChild(m_pRoot, > HXASXElementEntry | HXASXElementEntryRef | > HXASXElementRepeat, > i, &pEntry); > if (SUCCEEDED(retVal)) > { > if (pEntry->GetType() == HXASXElementRepeat) > { > ulRepeatEntries += GetNumChildren(pEntry, > HXASXElementEntry | HXASXElementEntryRef); > } > } > } > } > > UINT32 ulTotalEntries = ulAllEntries + ulRepeatEntries - ulNumRepeats; > > return ulTotalEntries; > } > > Similarly, you could create a GetIthEntryWithRepeat() function which > returned the i-th entry. If you had the following: > > <asx> > <entry><ref href="A.wmv"/></entry> > <repeat count="3"> > <entry><ref href="B.wmv"/></entry> > <entry><ref href="C.wmv"/></entry> > </repeat> > <entry><ref href="D.wmv"/></entry> > </asx> > > Then GetNumEntriesWithRepeat() would return 8. > And GetIthEntryWithRepeat(i,) would return: > > Entry i > ------------------------------------ > 0 A.wmv > 1 B.wmv (1st time through) > 2 C.wmv (1st time through) > 3 B.wmv (2nd time through) > 4 C.wmv (2nd time through) > 5 B.wmv (3rd time through) > 6 C.wmv (3rd time through) > 7 D.wmv > > > Thanks, > > Eric > > ======================================= > Eric Hyche (ehyche at real.com) > Principal Engineer > RealNetworks, Inc. > > >>-----Original Message----- >>From: Deepak Jain [mailto:deepakj at real.com] >>Sent: Tuesday, December 16, 2008 7:08 AM >>To: ehyche at real.com >>Cc: datatype-dev at helixcommunity.org >>Subject: Re: [datatype-dev] CR: Changes to Support REPEAT Tag in asxrender >> >>Hi Eric, >> >>Attached please find the updated diff with count attribute support. >> >>Thanks, >>Deepak Jain >> >>Eric Hyche wrote: >> >> If we are going to add support for the <REPEAT> element, then >> we need to support the "count" attribute as well. I don't >> think we should just make it so that it only repeats >> once. >> >> The MSDN documentation for the "count" attribute says this: >> >> "If no COUNT parameter is defined, the content in the associated >> ENTRY and ENTRYREF elements repeats an infinite number of times." >> >> We should probably set some sort of maximum number of repeats >> and never go above that. So if the "count" attribute is >> not specified, or the value of the count attribute is greater >> than this max, then we only repeat the maximum number of times. >> >> Also, we need to make sure we support multiple elements in >> the <REPEAT> element, like this: >> >> <repeat count="3"> >> <entry> >> <ref href="clip1.wmv"/> >> </entry> >> <entry> >> <ref href="clip2.wmv"/> >> </entry> >> </repeat> >> >> Then this would play like this: >> >> clip1.wmv >> clip2.wmv >> clip1.wmv >> clip2.wmv >> clip1.wmv >> clip2.wmv >> >> >> Eric >> >> ======================================= >> Eric Hyche (ehyche at real.com) >> Principal Engineer >> RealNetworks, Inc. >> >> >> >> >> -----Original Message----- >> From: datatype-dev-bounces at helixcommunity.org [mailto:datatype-dev- >>bounces at helixcommunity.org] On >> Behalf Of Deepak Jain >> Sent: Friday, December 12, 2008 6:15 AM >> To: datatype-dev at helixcommunity.org >> Subject: [datatype-dev] CR: Changes to Support REPEAT Tag in asxrender >> >> Synopsis: >> Changes to Support REPEAT Tag in asxrender >> >> Overview: >> Currently there is not support in asxrender to support the REPEAT tag in >> the asx file. If >>we have a >> asx file with REPEAT tag, we cannot play that file. >> The changes are added to support the REPEAT tag. >> Currently the REPEAT is supported once irrespective of the COUNT >> attribute or not. All the >>refs within >> REPEAT tag are added once. >> >> Files Added: >> None >> >> Files Removed >> None >> >> Files Modified: >> \datatype\wm\asx\renderer\asxrender.cpp >> >> Image Size and Heap Use impact (Client -Only): >> None. >> >> Platforms and Profiles Affected: >> None >> >> Distribution Libraries Affected: >> None >> >> Distribution library impact and planned action: >> None >> >> Platforms and Profiles Build Verified: >> BIF branch -> hxclient_3_1_0_atlas_restricted >> Target(s) -> splay >> Profile -> helix-client-all-defines >> SYSTEM_ID -> win32-i386-vc7 >> >> Branch: >> Atlas310 >> >> Files Attached: >> asxrender_diff.txt >> >> Thanks, >> Deepak Jain >> >> >> >> >> >> >> > > > > _______________________________________________ > Datatype-dev mailing list > Datatype-dev at helixcommunity.org > http://lists.helixcommunity.org/mailman/listinfo/datatype-dev -------------- next part -------------- Index: asxelement.cpp =================================================================== RCS file: /cvsroot/datatype/wm/asx/renderer/asxelement.cpp,v retrieving revision 1.4.2.1 diff -u -r1.4.2.1 asxelement.cpp --- asxelement.cpp 22 Dec 2008 06:10:42 -0000 1.4.2.1 +++ asxelement.cpp 22 Dec 2008 12:59:04 -0000 @@ -964,7 +964,7 @@ CHXASXElementRepeat::CHXASXElementRepeat(IUnknown* pContext, HXASXElement eType, IHXValues* pAttr, UINT32 ulLine, UINT32 ulCol, CHXASXElement* pParent, HX_RESULT* pRet) : CHXASXElement(pContext, eType, pAttr, ulLine, ulCol, pParent, pRet) - , m_ulCount(0xFFFFFFFF) + , m_ulCount(MAX_REPEAT_COUNT) { if (m_pAttr && pRet) { Index: asxelement.h =================================================================== RCS file: /cvsroot/datatype/wm/asx/renderer/asxelement.h,v retrieving revision 1.2 diff -u -r1.2 asxelement.h --- asxelement.h 6 Jul 2007 22:02:11 -0000 1.2 +++ asxelement.h 22 Dec 2008 12:59:04 -0000 @@ -57,6 +57,8 @@ #include "hxstring.h" #include "asxtypes.h" +#define MAX_REPEAT_COUNT 100 //Max Number of counts for a <repeat> tag. + typedef struct _HXASXElementNamePair { HXASXElement m_eType; Index: asxrender.cpp =================================================================== RCS file: /cvsroot/datatype/wm/asx/renderer/asxrender.cpp,v retrieving revision 1.4.2.2 diff -u -r1.4.2.2 asxrender.cpp --- asxrender.cpp 22 Dec 2008 05:00:07 -0000 1.4.2.2 +++ asxrender.cpp 22 Dec 2008 12:59:05 -0000 @@ -72,6 +72,8 @@ , m_pStack(NULL) , m_pParser(NULL) , m_bDeprecated(FALSE) + , m_bRepeat(FALSE) + , m_ulRepeatCount(NULL) { HXLOGL4(HXLOG_ASXR, "CON CASXRenderer this=%p", this); } @@ -531,6 +533,11 @@ { UINT32 ulRet = GetNumChildren(m_pRoot, HXASXElementEntry | HXASXElementEntryRef); + if(m_bRepeat) + { + ulRet = GetNumEntriesWithRepeat(); + } + HXLOGL4(HXLOG_ASXR, "%p::CASXRenderer::GetNumEntries() returns %lu", this, ulRet); if(m_bDeprecated) { @@ -539,6 +546,37 @@ return ulRet; } +UINT32 CASXRenderer:: GetNumEntriesWithRepeat() +{ + HX_RESULT retVal = HXR_OK; + UINT32 ulAllEntries = GetNumChildren(m_pRoot, + HXASXElementEntry | HXASXElementEntryRef | HXASXElementRepeat); + UINT32 ulNumRepeats = GetNumChildren(m_pRoot, HXASXElementRepeat); + UINT32 ulRepeatEntries = 0; + if (ulNumRepeats) + { + UINT32 i = 0; + for (i = 0; i < ulAllEntries; i++) + { + CHXASXElement* pEntry = NULL; + retVal = GetIthChild(m_pRoot, + HXASXElementEntry | HXASXElementEntryRef | HXASXElementRepeat, + i, &pEntry); + if (SUCCEEDED(retVal)) + { + if (pEntry->GetType() == HXASXElementRepeat) + { + ulRepeatEntries += GetNumChildren(pEntry, HXASXElementEntry | HXASXElementEntryRef); + } + } + } + } + + UINT32 ulTotalEntries = ulAllEntries + m_ulRepeatCount * ulRepeatEntries - ulNumRepeats; + + return ulTotalEntries; +} + HX_RESULT CASXRenderer::GetEntryMetaData(UINT32 ulEntryIndex, REF(IHXValues*) rpData) { HX_RESULT retVal = HXR_FAIL; @@ -744,7 +782,7 @@ pDstCh++; bCopyChar = FALSE; } - else if ((bInsideBrackets && bInsideQuotes) || !bInsideBrackets) + else if (bInsideBrackets && bInsideQuotes) { // Is this a character which needs to be escaped? if (strchr(pszEscChars, *pSrcCh)) @@ -1007,6 +1045,16 @@ { ulRet++; } + else if (pChild->GetType() == HXASXElementRepeat) + { + m_bRepeat = TRUE; + m_ulRepeatCount = ((CHXASXElementRepeat*)(pChild))->GetCount(); + if (m_ulRepeatCount > MAX_REPEAT_COUNT) + { + m_ulRepeatCount = MAX_REPEAT_COUNT; + } + ulRet+=GetNumChildren(pChild, HXASXElementEntry | HXASXElementEntryRef); + } HX_RELEASE(pChild); rv = pParent->GetNextChild(&pChild); } @@ -1051,6 +1099,22 @@ // Increment the child index ulChildIndex++; } + else if (pChild->GetType() == HXASXElementRepeat) + { + UINT32 NumChildren = GetNumChildren(pChild, HXASXElementEntry | HXASXElementEntryRef); + UINT32 RepeatCount = ((CHXASXElementRepeat*)(pChild))->GetCount(); + UINT32 j = i-ulChildIndex; + if((RepeatCount*NumChildren) > j) + { + j%=NumChildren; + retVal = GetIthChild(pChild, HXASXElementEntry | HXASXElementEntryRef, j, ppChild); + break; + } + else + { + ulChildIndex += RepeatCount*NumChildren; + } + } HX_RELEASE(pChild); rv = pParent->GetNextChild(&pChild); } Index: asxrender.h =================================================================== RCS file: /cvsroot/datatype/wm/asx/renderer/asxrender.h,v retrieving revision 1.3.2.2 diff -u -r1.3.2.2 asxrender.h --- asxrender.h 9 Dec 2008 12:51:33 -0000 1.3.2.2 +++ asxrender.h 22 Dec 2008 12:59:05 -0000 @@ -127,6 +127,8 @@ IHXXMLParser* m_pParser; CHXSimpleList m_Deprecatedlist; HXBOOL m_bDeprecated; + HXBOOL m_bRepeat; + UINT32 m_ulRepeatCount; // These are methods that the derived class must implement virtual HX_RESULT ParseDeprecatedMetaFile(IHXBuffer* pBuffer); @@ -139,7 +141,8 @@ virtual UINT32 GetNumEntryRefs(UINT32 ulEntryIndex); virtual HX_RESULT GetEntryRefMetaData(UINT32 ulEntryIndex, UINT32 ulRefIndex, REF(IHXValues*) rpData); virtual void EntryStarted(UINT32 ulEntryIndex); - + UINT32 GetNumEntriesWithRepeat(); + HX_RESULT GetIthChildWithRepeat(CHXASXElement* pParent, UINT32 ulType, UINT32 i, CHXASXElement** ppChild); // These are internal methods for CASXRenderer HX_RESULT PreProcessASX(IHXBuffer* pSrc, REF(IHXBuffer*) rpDst); HX_RESULT SetContentAsBufferProperty(IHXValues* pData, const char* pszName, CHXASXElement* pElem);