[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-common-dev
Subject: [Common-dev] RE: CR: Changed common registry to work w/o context
From: "Sujeet Kharkar" <skharkar () real ! com>
Date: 2009-02-19 5:15:08
Message-ID: 004901c99251$08979d50$9b24fea9 () sujeetlaptop
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Yes SAP Manager is Announcement Protocol Manager.
I will change define to HELIX_CONFIG_REMBROADCAST_OWN_CONTEXT and add your's
and Eric's recommendation to bug I log for doing solution A later.
Thanks,
Sujeet
_____
From: Milko Boic [mailto:milko@real.com]
Sent: Tuesday, February 17, 2009 10:51 PM
To: Sujeet Kharkar; common-dev@helixcommunity.org
Subject: Re: CR: Changed common registry to work w/o context with
HELIX_FEATURE_CLIENT defined.
At 08:25 PM 2/17/2009, Sujeet Kharkar wrote:
Synopsis
============
Fix crash in rembrdcst.
Suggested Reviewer: Eric Hyche.
Branches
=========
Head
Description
==============
Rembrdcst library uses it's own context, if no context is passed to it.
Producer is not passing context to rembrcst, in which case internal context
loaded by rembrdcst which uses registry, which requires that context is
available if HELIX_FEATURE_CLIENT is defined.
Since Producer now has HELIX_FEATURE_CLIENT defined, this causes a crash.
Possible Fixes:
A> Use media platform as context and remove context from statically linking
to rembrdcst.(At least ifdef it.)
This will also involve adding support for IID_IHXSapManager and
IID_IHXErrorMessages to media platform.
What is SapManager? Is this Session Announcement Protocol Manager? What is
the role of the object behind this interface and why would it be a singleton
for the entire media platform?
Idea of supporting IHXErrorMessages in media platform is a good one. It
would need to be done by providing IHXErrorSinkControl implementation there.
One implementation exists in media player and could be moved there with some
modifications.
However, this implementation needs to be done with consideration of child
platform as follows:
- Each child platform needs to implement own ErrorSinkControl to allow
monitoring errors only on the child instance
- Each child platform automatically adds its parent as one of the error
sinks in order to allow parent error sink to monitor child errors
Player would still use own ErrorSinkControl but now created from media
platform CCF and adding media platform as one of its sinks.
This would enable media platform to act as generic collector of error
messages for any object but also provide an easy way for any object to
create its own ErrorSinkControl.
However, given that SapManager is needed in the context may be an indication
that more specific context than media platform should be passed in. Perhaps
that of the encoding job object - which would then create
IHXErrorSinkControl and SapManager.
Estimate: 2 days.
B>
Also check if new defined HELIX_NOT_USING_COMMON_CONTEXT is set, which will
be set by producer.
While this may be a temporary define, it needs to be more specific.
E.g.:
HELIX_CONFIG_REMBROADCAST_OWN_CONTEXT
Naming guidelines for global defines are as follows:
- Names start with either HELIX_FEATURE or HELIX_CONFIG
- Names are structure from more general to more specific moving from left to
right
- Resulting name must be descriptive enough to give a good idea of where it
is used and the purposes without looking up the code where it is used.
Milko
Due to less time I will log a bug to do A> and have implemented B for now.
Files affected
==================
/cvsroot/common/util/commreg.cpp
/cvsroot/common/util/asmrulep.cpp
Testing
=================
Verified that rembrdcst does not crash.
QA Hints
=============
Test various modes of rembrote broadcast.
Diff
=========
Index: commreg.cpp
===================================================================
RCS file: /cvsroot/common/util/commreg.cpp,v
retrieving revision 1.20
diff -u -r1.20 commreg.cpp
--- commreg.cpp 6 Jul 2007 20:39:16 -0000 1.20
+++ commreg.cpp 18 Feb 2009 03:55:24 -0000
@@ -2085,7 +2085,7 @@
return CreateAndSetBufferCCF(prop_name,
(UCHAR*)p->get_key_str(),
p->get_key_str_len(),
m_pContext);
}
-#ifndef HELIX_FEATURE_CLIENT
+#if !defined(HELIX_FEATURE_CLIENT) ||
defined(HELIX_NOT_USING_COMMON_CONTEXT)
prop_name = new CHXBuffer;
prop_name->Set((const unsigned char *)p->get_key_str(),
p->get_key_str_len());
@@ -2318,7 +2318,7 @@
CreateValuesCCF(pValues, m_pContext);
}
// Client should always provide m_pContext
-#ifndef HELIX_FEATURE_CLIENT
+#if !defined(HELIX_FEATURE_CLIENT) ||
defined(HELIX_NOT_USING_COMMON_CONTEXT)
else
{
pValues = new CHXHeader;
Index: asmrulep.cpp
===================================================================
RCS file: /cvsroot/common/util/asmrulep.cpp,v
retrieving revision 1.27
diff -u -r1.27 asmrulep.cpp
--- asmrulep.cpp 6 May 2008 18:45:27 -0000 1.27
+++ asmrulep.cpp 18 Feb 2009 03:55:24 -0000
@@ -1584,7 +1584,7 @@
return HXR_OUTOFMEMORY;
}
}
-#ifndef HELIX_FEATURE_CLIENT
+#if !defined(HELIX_FEATURE_CLIENT) ||
defined(HELIX_NOT_USING_COMMON_CONTEXT)
// Client should always provide m_pContext
else
{
Index: commreg.cpp
[Attachment #5 (text/html)]
<html xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv=Content-Type content="text/html; charset=us-ascii">
<meta name=Generator content="Microsoft Word 11 (filtered medium)">
<!--[if !mso]>
<style>
v\:* {behavior:url(#default#VML);}
o\:* {behavior:url(#default#VML);}
w\:* {behavior:url(#default#VML);}
.shape {behavior:url(#default#VML);}
</style>
<![endif]-->
<style>
<!--
/* Font Definitions */
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman";}
a:link, span.MsoHyperlink
{color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:Arial;
color:navy;}
@page Section1
{size:8.5in 11.0in;
margin:1.0in 1.25in 1.0in 1.25in;}
div.Section1
{page:Section1;}
-->
</style>
</head>
<body lang=EN-US link=blue vlink=purple>
<div class=Section1>
<p class=MsoNormal><font size=2 color=navy face=Arial><span style='font-size:
10.0pt;font-family:Arial;color:navy'>Yes SAP Manager is </span></font>Announcement
Protocol Manager.<o:p></o:p></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'><o:p> </o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'>I will change define to HELIX_CONFIG_REMBROADCAST_OWN_CONTEXT and add
your’s and Eric’s recommendation to bug I log for doing solution A
later.<o:p></o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'><o:p> </o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'>Thanks,<o:p></o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'>Sujeet<br>
<br>
<o:p></o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'><o:p> </o:p></span></font></p>
<p class=MsoNormal><font size=2 color=navy face=Arial><span style='font-size:
10.0pt;font-family:Arial;color:navy'><o:p> </o:p></span></font></p>
<div>
<div class=MsoNormal align=center style='text-align:center'><font size=3
face="Times New Roman"><span style='font-size:12.0pt'>
<hr size=2 width="100%" align=center tabindex=-1>
</span></font></div>
<p class=MsoNormal><b><font size=2 face=Tahoma><span style='font-size:10.0pt;
font-family:Tahoma;font-weight:bold'>From:</span></font></b><font size=2
face=Tahoma><span style='font-size:10.0pt;font-family:Tahoma'> Milko Boic
[mailto:milko@real.com] <br>
<b><span style='font-weight:bold'>Sent:</span></b> Tuesday, February 17, 2009
10:51 PM<br>
<b><span style='font-weight:bold'>To:</span></b> Sujeet Kharkar;
common-dev@helixcommunity.org<br>
<b><span style='font-weight:bold'>Subject:</span></b> Re: CR: Changed common
registry to work w/o context with HELIX_FEATURE_CLIENT \
defined.</span></font><o:p></o:p></p>
</div>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'><o:p> </o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'>At 08:25 PM 2/17/2009, Sujeet Kharkar wrote:<br>
<br>
<o:p></o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'>Synopsis<br>
============<br>
Fix crash in rembrdcst.<br>
Suggested Reviewer: Eric Hyche.<br>
<br>
Branches<br>
=========<br>
Head<br>
<br>
Description<br>
==============<br>
Rembrdcst library uses it's own context, if no context is passed to it. <br>
<br>
Producer is not passing context to rembrcst, in which case internal context<br>
loaded by rembrdcst which uses registry, which requires that context is<br>
available if HELIX_FEATURE_CLIENT is defined. <br>
Since Producer now has HELIX_FEATURE_CLIENT defined, this causes a crash.<br>
<br>
Possible Fixes:<br>
A> Use media platform as context and remove context from statically linking<br>
to rembrdcst.(At least ifdef it.) <br>
This will also involve adding support for IID_IHXSapManager and<br>
IID_IHXErrorMessages to media platform. <o:p></o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'><br>
What is SapManager? Is this Session Announcement Protocol Manager?
What is the role of the object behind this interface and why would it be a
singleton for the entire media platform?<br>
<br>
Idea of supporting IHXErrorMessages in media platform is a good one. It
would need to be done by providing </span></font><font size=2 color=green
face=Arial><span style='font-size:10.0pt;font-family:Arial;color:green'>IHXErrorSinkControl
</span></font>implementation there. One implementation exists in media
player and could be moved there with some modifications.<br>
However, this implementation needs to be done with consideration of child
platform as follows:<br>
- Each child platform needs to implement own ErrorSinkControl to allow
monitoring errors only on the child instance<br>
- Each child platform automatically adds its parent as one of the error sinks
in order to allow parent error sink to monitor child errors<br>
<br>
Player would still use own ErrorSinkControl but now created from media platform
CCF and adding media platform as one of its sinks.<br>
<br>
This would enable media platform to act as generic collector of error messages
for any object but also provide an easy way for any object to create its own
ErrorSinkControl.<br>
<br>
However, given that SapManager is needed in the context may be an indication
that more specific context than media platform should be passed in.
Perhaps that of the encoding job object - which would then create <font size=2
color=green face=Arial><span style='font-size:10.0pt;font-family:Arial;
color:green'>IHXErrorSinkControl </span></font>and SapManager.<br>
<br>
<br>
<br>
<o:p></o:p></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'>Estimate: 2 days.<br>
<br>
B><br>
Also check if new defined HELIX_NOT_USING_COMMON_CONTEXT is set, which will<br>
be set by producer.<o:p></o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'><br>
While this may be a temporary define, it needs to be more specific.<br>
E.g.:<br>
HELIX_CONFIG_REMBROADCAST_OWN_CONTEXT<br>
<br>
Naming guidelines for global defines are as follows:<br>
- Names start with either HELIX_FEATURE or HELIX_CONFIG<br>
- Names are structure from more general to more specific moving from left to
right<br>
- Resulting name must be descriptive enough to give a good idea of where it is
used and the purposes without looking up the code where it is used.<br>
<br>
Milko<br>
<br>
<br>
<br>
<o:p></o:p></span></font></p>
<p class=MsoNormal><font size=3 face="Times New Roman"><span style='font-size:
12.0pt'>Due to less time I will log a bug to do A> and have implemented B
for now.<br>
<br>
Files affected<br>
==================<br>
/cvsroot/common/util/commreg.cpp<br>
/cvsroot/common/util/asmrulep.cpp<br>
<br>
Testing<br>
=================<br>
Verified that rembrdcst does not crash.<br>
<br>
QA Hints<br>
=============<br>
Test various modes of rembrote broadcast.<br>
<br>
Diff<br>
=========<br>
<br>
Index: commreg.cpp<br>
===================================================================<br>
RCS file: /cvsroot/common/util/commreg.cpp,v<br>
retrieving revision 1.20<br>
diff -u -r1.20 commreg.cpp<br>
--- commreg.cpp<x-tab> </x-tab>6 Jul 2007 20:39:16 \
-0000<x-tab> </x-tab>1.20<br>
+++ commreg.cpp<x-tab> </x-tab>18 Feb 2009 03:55:24 -0000<br>
@@ -2085,7 +2085,7 @@<br>
<x-tab> </x-tab><x-tab>
</x-tab>return
CreateAndSetBufferCCF(prop_name,<br>
(UCHAR*)p->get_key_str(),<br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab><x-tab>
</x-tab><x-tab>
</x-tab>
p->get_key_str_len(),<br>
m_pContext);<br>
<x-tab> </x-tab>
}<br>
-#ifndef HELIX_FEATURE_CLIENT<br>
+#if !defined(HELIX_FEATURE_CLIENT) ||<br>
defined(HELIX_NOT_USING_COMMON_CONTEXT)<br>
<x-tab> </x-tab>
prop_name = new CHXBuffer;<br>
<x-tab> </x-tab>
prop_name->Set((const unsigned char
*)p->get_key_str(), <br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab>
p->get_key_str_len());<br>
@@ -2318,7 +2318,7 @@<br>
<x-tab> </x-tab>
CreateValuesCCF(pValues, m_pContext);<br>
}<br>
// Client should always provide m_pContext<br>
-#ifndef HELIX_FEATURE_CLIENT<br>
+#if !defined(HELIX_FEATURE_CLIENT) ||<br>
defined(HELIX_NOT_USING_COMMON_CONTEXT)<br>
else<br>
{<br>
<x-tab> </x-tab>pValues = new
CHXHeader;<br>
<br>
Index: asmrulep.cpp<br>
===================================================================<br>
RCS file: /cvsroot/common/util/asmrulep.cpp,v<br>
retrieving revision 1.27<br>
diff -u -r1.27 asmrulep.cpp<br>
--- asmrulep.cpp<x-tab> </x-tab>6
May 2008 18:45:27 -0000<x-tab> </x-tab>1.27<br>
+++ asmrulep.cpp<x-tab> </x-tab>18
Feb 2009 03:55:24 -0000<br>
@@ -1584,7 +1584,7 @@<br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab><x-tab>
</x-tab>return HXR_OUTOFMEMORY;<br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab> }<br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab>}<br>
-#ifndef HELIX_FEATURE_CLIENT<br>
+#if !defined(HELIX_FEATURE_CLIENT) ||<br>
defined(HELIX_NOT_USING_COMMON_CONTEXT)<br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab>// Client should always
provide m_pContext<br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab>else<br>
<x-tab> </x-tab><x-tab>
</x-tab><x-tab>
</x-tab>{<br>
Index: commreg.cpp<o:p></o:p></span></font></p>
</div>
</body>
<br>
</html>
_______________________________________________
Common-dev mailing list
Common-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/common-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic