[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-video-dev
Subject: Re: [Video-dev] surfaces / sites
From: Greg Wright <gwright () real ! com>
Date: 2005-03-18 21:32:56
Message-ID: 423B4908.7000102 () real ! com
[Download RAW message or body]
Sounds good.
I have comitted the change to HEAD and 150Cay.
--greg.
Greg Stacey wrote:
> Considering the implementations of _MinimalBlt are as follows:
>
> Baseroot.cpp - empty
> void CBaseRootSurface::_MinimalBlt(HXxRect&, HXxRect&)
> {}
>
>
> Winroot.cpp - empty
> void CWinBaseRootSurface::_MinimalBlt(HXxRect& src, HXxRect& dst)
> {
> }
>
> Unix - already tested
>
> Mac is the only implementation that needs testing. And a quick check of the code \
> shows that the parameter is never actually used in the function, so I wouldn't \
> think much is needed.
> I'm inclined to leave my original patch that fixes the non-shm side of \
> CUnixRootSurf::_MinimalBlt as is, and submit a separate patch to remove the now \
> unused parameter. What do you think?
> Greg Stacey
> Principal Consultant
> Advanced Technologies Integration
>
>
> > -----Original Message-----
> > From: Greg Wright [mailto:gwright@real.com]
> > Sent: Friday, March 18, 2005 1:53 PM
> > To: Greg Stacey
> > Cc: video-dev@helixcommunity.org
> > Subject: Re: [Video-dev] surfaces / sites
> >
> > Greg Stacey wrote:
> >
> > > But if I refactored to remove srcRect, I could get rid of this comment in \
> > > baseroot.cpp
> > > //XXXgfw this is weird. We only want to redraw the Dest
> > > //Rect right? The src is mostly always zero..
> > > if( !m_pSite->IsCompositionLocked() && returnVal != -1)
> > > _MinimalBlt(rSrcRect, rDestRect);
> > > }
> > >
> > > Yours? =)
> >
> > Yup, caught red handed. :)
> >
> > However, making this change now means changing it on *all* platforms
> > with all the testing that goes along with it. If you have enough
> > time then go ahead and make changes to all *root.cpp files for all
> > platforms. Can you test it on at least 2 platforms (windows being one
> > would be good). If so, then I can help on the rest.
> >
> > We can at least make this change for HEAD. I think we can get it in
> > 150Cay also once we know we haven't broken any builds with it.
> >
> > --greg.
> >
> >
> >
> >
> > > Greg Stacey
> > > Principal Consultant
> > > Advanced Technologies Integration
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg Wright [mailto:gwright@real.com]
> > > > Sent: Friday, March 18, 2005 12:36 PM
> > > > To: Greg Stacey
> > > > Cc: video-dev@helixcommunity.org
> > > > Subject: Re: [Video-dev] surfaces / sites
> > > >
> > > > Greg Stacey wrote:
> > > >
> > > >
> > > > > I submitted a CR for the UseShm bug, but I have a question before I submit \
> > > > > a CR for the
> >
> > _MinimalBlt
> >
> > > > problem. The code change is to
> > > >
> > > >
> > > > > use destRect as the src parameter in the call to XPutImage as it does in \
> > > > > the UseShm side of the
> >
> > if.
> >
> > > > Once this change is made, the
> > > >
> > > >
> > > > > srcRect parameter is no longer used in the function. I did a quick check \
> > > > > and it is not used in
> >
> > any
> >
> > > > of the other platform
> > > >
> > > >
> > > > > implementations either. Should I do a quick refactoring of this function \
> > > > > to remove the srcRect
> > > >
> > > > parameter?
> > > >
> > > >
> > > > Lets leave it along for now and just fix up the non-shm side of things.
> > > >
> > > > --greg.
> > > >
> > > >
> > > >
> > > >
> > > > > Greg Stacey
> > > > > Principal Consultant
> > > > > Advanced Technologies Integration
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg Wright [mailto:gwright@real.com]
> > > > > > Sent: Thursday, March 17, 2005 6:28 PM
> > > > > > To: Greg Stacey
> > > > > > Cc: 'Ryan Gammon'; video-dev@helixcommunity.org
> > > > > > Subject: Re: [Video-dev] surfaces / sites
> > > > > >
> > > > > > Nice work Greg. Looks like you found a couple good bugs for
> > > > > > sure.
> > > > > >
> > > > > > --greg.
> > > > > >
> > > > > >
> > > > > > Greg Stacey wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > Ok. I've finally figured out what's going on here. There are actually \
> > > > > > > two bugs in the code,
> >
> > and
> >
> > > > > > I'll submit patch(es) for them
> > > > > >
> > > > > >
> > > > > >
> > > > > > > tomorrow(about to call it a day here).
> > > > > > >
> > > > > > > I thought that somewhere upstream the src coordinates were not getting \
> > > > > > > set properly and this was
> > > > > >
> > > > > > causing the problem. The actual
> > > > > >
> > > > > >
> > > > > >
> > > > > > > case is that the call should not be using the src coordinates at all.
> > > > > > >
> > > > > > > > From unixroot.cpp:
> > > > > > > void CUnixRootSurf::_MinimalBlt(HXxRect& srcRect, HXxRect& destRect)
> > > > > > > {
> > > > > > > Window win = m_window;
> > > > > > > HX_ASSERT(win);
> > > > > > >
> > > > > > > if (m_bUseShm)
> > > > > > > {
> > > > > > > XLockDisplay(m_pDisplay);
> > > > > > > XShmPutImage(m_pDisplay,
> > > > > > > win,
> > > > > > > m_GC,
> > > > > > > m_pXImage,
> > > > > > > destRect.left,
> > > > > > > destRect.top,
> > > > > > > destRect.left,
> > > > > > > destRect.top,
> > > > > > > destRect.right - destRect.left,
> > > > > > > destRect.bottom - destRect.top,
> > > > > > > False
> > > > > > > );
> > > > > > > XUnlockDisplay(m_pDisplay);
> > > > > > > }
> > > > > > > else
> > > > > > > {
> > > > > > > XLockDisplay(m_pDisplay);
> > > > > > > XPutImage(m_pDisplay,
> > > > > > > win,
> > > > > > > m_GC,
> > > > > > > m_pXImage,
> > > > > > > srcRect.left,
> > > > > > > srcRect.top,
> > > > > > > destRect.left,
> > > > > > > destRect.top,
> > > > > > > destRect.right - destRect.left,
> > > > > > > destRect.bottom - destRect.top);
> > > > > > > XUnlockDisplay(m_pDisplay);
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > If I had looked at the code in question more closely I probably would \
> > > > > > > have figured this out
> > > >
> > > > sooner.
> > > >
> > > >
> > > > > > In the bUseShm section the call
> > > > > >
> > > > > >
> > > > > >
> > > > > > > to XPutImage uses destRect left and top, for the src parameters while \
> > > > > > > the !bUseShm section uses
> > > >
> > > > the
> > > >
> > > >
> > > > > > actual srcRect. The desired
> > > > > >
> > > > > >
> > > > > >
> > > > > > > functionality here is to have the dest and src parameters match as in \
> > > > > > > the bUseShm side.
> > > > > > > I found the other bug when I asked myself why HPUX wasn't using shm. \
> > > > > > > It should be but there is
> >
> > a
> >
> > > > > > bug in shmhelp.cpp:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > if(zm_bUseShm)
> > > > > > > {
> > > > > > > char *disp = getenv("DISPLAY");
> > > > > > > HX_ASSERT( disp != NULL && "Your display variable isn't set!" );
> > > > > > > if(!disp)
> > > > > > > {
> > > > > > > zm_bUseShm = FALSE;
> > > > > > > }
> > > > > > > else if( disp[0]!=':' && disp[0]!='0' )
> > > > > > > {
> > > > > > > //See if our machine name is in the dipsplay string. struct \
> > > > > > > utsname inf; uname(&inf);
> > > > > > > if( (strlen(disp)<=strlen(inf.machine)) ||
> > > > > > > (strncmp(disp, inf.machine, strlen(inf.machine))) ) {
> > > > > > > zm_bUseShm = FALSE;
> > > > > > > }
> > > > > > >
> > > > > > > In this case inf.machine is the hardware specification. The code \
> > > > > > > should be using inf.nodename
> > > > > >
> > > > > > instead. Otherwise the DISPLAY
> > > > > >
> > > > > >
> > > > > >
> > > > > > > variable will not match properly.
> > > > > > >
> > > > > > > This is easy to verify. On Linux manually change your DISPLAY \
> > > > > > > environment variable from :0.0 to
> > > > > >
> > > > > > machine_name:0.0, where
> > > > > >
> > > > > >
> > > > > >
> > > > > > > machine_name is the name of you box from 'uname -n'. This will trigger \
> > > > > > > the second bug and the
> > > > > >
> > > > > > player will not use shm. This will,
> > > > > >
> > > > > >
> > > > > >
> > > > > > > in turn, cause the non-shm side of CUnixRootSurf::_MinimalBlt to \
> > > > > > > execute and you will see a bad
> > > > > >
> > > > > > layout in the following URL:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > http://realmedia.uic.edu/ramgen/itltv/bbintro.30jan02.smil
> > > > > > >
> > > > > > > Ryan and Greg - thanks for the help.
> > > > > > >
> > > > > > > I'll post a patch tomorrow. Should I combine them or submit them as \
> > > > > > > separate patches?
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Greg Stacey
> > > > > > > Principal Consultant
> > > > > > > Advanced Technologies Integration
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ryan Gammon [mailto:rgammon@real.com]
> > > > > > > > Sent: Thursday, March 17, 2005 12:05 PM
> > > > > > > > To: Greg Stacey
> > > > > > > > Cc: video-dev@helixcommunity.org
> > > > > > > > Subject: Re: [Video-dev] surfaces / sites
> > > > > > > >
> > > > > > > > Greg Stacey wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > Ryan - If you could do a control-click and repost yours that would \
> > > > > > > > > be great.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Attached.
> > > > > > > >
> > > > > > > > --
> > > > > > > > Ryan Gammon
> > > > > > > > rgammon@real.com
> > > > > > > > Developer for Helix Player
> > > > > > > > https://player.helixcommunity.org
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > Video-dev mailing list
> > > > > > > Video-dev@helixcommunity.org
> > > > > > > http://lists.helixcommunity.org/mailman/listinfo/video-dev
> > > > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > Video-dev mailing list
> > > > > Video-dev@helixcommunity.org
> > > > > http://lists.helixcommunity.org/mailman/listinfo/video-dev
> > > > >
> > >
> > >
> > >
> > >
> > >
>
>
>
>
> _______________________________________________
> Video-dev mailing list
> Video-dev@helixcommunity.org
> http://lists.helixcommunity.org/mailman/listinfo/video-dev
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic