Re: [opensource-dev] Review Request: (STORM-380) There is a little delay in sound when gesture first time played

2011-03-28 Thread Oz Linden


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 536-537
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line536
 
  If the iterator will not be used outside the loop, please declare it 
  with the assignment in the for's braces.

I don't see any reason to make that distinction.


- Oz


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/231/#review506
---


On March 25, 2011, 5:33 p.m., Seth ProductEngine wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://codereview.secondlife.com/r/231/
 ---
 
 (Updated March 25, 2011, 5:33 p.m.)
 
 
 Review request for Viewer.
 
 
 Summary
 ---
 
 First pass implementation of syncing the animations and sounds before the 
 gesture starts playing.
 The actual playing of animations and sounds of a gesture starts only when all 
 needed animations and sound files are loaded into viewer cache. This reduces 
 the delay between animations and sounds meant to be played simultaneously but 
 may increase the delay between the moment a gesture is triggered and the 
 moment it starts playing.
 
 
 This addresses bug STORM-380.
 http://jira.secondlife.com/browse/STORM-380
 
 
 Diffs
 -
 
   indra/newview/llgesturemgr.h 6c15f820c3b9 
   indra/newview/llgesturemgr.cpp 6c15f820c3b9 
 
 Diff: http://codereview.secondlife.com/r/231/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Seth
 


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: (STORM-380) There is a little delay in sound when gesture first time played

2011-03-28 Thread Oz Linden

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/231/#review508
---



indra/newview/llgesturemgr.cpp
http://codereview.secondlife.com/r/231/#comment381

I think that there's a race condition here.  

Wouldn't it be better to insert the id into the mLoadingAssets before 
requesting it, so that it is sure to be in the list if the onAssetLoadComplete 
is called immediately?




indra/newview/llgesturemgr.cpp
http://codereview.secondlife.com/r/231/#comment382

Same possible race as above


- Oz


On March 25, 2011, 5:33 p.m., Seth ProductEngine wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://codereview.secondlife.com/r/231/
 ---
 
 (Updated March 25, 2011, 5:33 p.m.)
 
 
 Review request for Viewer.
 
 
 Summary
 ---
 
 First pass implementation of syncing the animations and sounds before the 
 gesture starts playing.
 The actual playing of animations and sounds of a gesture starts only when all 
 needed animations and sound files are loaded into viewer cache. This reduces 
 the delay between animations and sounds meant to be played simultaneously but 
 may increase the delay between the moment a gesture is triggered and the 
 moment it starts playing.
 
 
 This addresses bug STORM-380.
 http://jira.secondlife.com/browse/STORM-380
 
 
 Diffs
 -
 
   indra/newview/llgesturemgr.h 6c15f820c3b9 
   indra/newview/llgesturemgr.cpp 6c15f820c3b9 
 
 Diff: http://codereview.secondlife.com/r/231/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Seth
 


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] Review Request: VWR-20801 Implement SOCKS 5 Proxy for the viewer

2011-03-28 Thread Robin Cornelius

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/232/
---

Review request for Viewer.


Summary
---

VWR-20801 - Add ability to use SOCKS 5 proxy to the viewer. This allows the UDP 
and/or the http requests to be sent via a SOCKS 5 proxy. This also allows http 
proxies to be used for other http operations such as caps etc as required. All 
the proxy settings have been unified on a single proxy floater accessable from 
preferences. 


This addresses bug VWR-20801.
http://jira.secondlife.com/browse/VWR-20801


Diffs
-

  indra/llmessage/CMakeLists.txt 65ff7415f171 
  indra/llmessage/llcurl.cpp 65ff7415f171 
  indra/llmessage/llpacketring.h 65ff7415f171 
  indra/llmessage/llpacketring.cpp 65ff7415f171 
  indra/llmessage/llsocks5.h PRE-CREATION 
  indra/llmessage/llsocks5.cpp PRE-CREATION 
  indra/llmessage/net.h 65ff7415f171 
  indra/llmessage/net.cpp 65ff7415f171 
  indra/newview/app_settings/settings.xml 65ff7415f171 
  indra/newview/llfloaterpreference.h 65ff7415f171 
  indra/newview/llfloaterpreference.cpp 65ff7415f171 
  indra/newview/llstartup.h 65ff7415f171 
  indra/newview/llstartup.cpp 65ff7415f171 
  indra/newview/llviewerfloaterreg.cpp 65ff7415f171 
  indra/newview/llxmlrpctransaction.cpp 65ff7415f171 
  indra/newview/skins/default/xui/en/floater_preferences_proxy.xml PRE-CREATION 
  indra/newview/skins/default/xui/en/notifications.xml 65ff7415f171 
  indra/newview/skins/default/xui/en/panel_preferences_setup.xml 65ff7415f171 

Diff: http://codereview.secondlife.com/r/232/diff


Testing
---

Verified login and in world interaction with proxy disabled, verified login and 
in world interactionvia socks 5 proxy. Code has been tested on Windows very 
recently and has also worked fine on linux, but i'm not currently in a position 
to retest that or Mac at all. Much more testing is needed to verify this does 
not break anything unexpectedly and also works as expected when enabled. To 
test requires a working socks 5 proxy.


Thanks,

Robin

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread Mike Chase
Is there a Linux build of V2 of any version that doesnt exhibit the 
annoying multi-second pauses that freeze the UI?  I find myself without 
any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both 
still have this issue.

How in the world did this every get past QA?  It really renders the 
viewer unusable. I've been using Imprudence the last few days which is 
nice but a huge shift in UI and I've actually gotten both used to and 
productive with the V2 paradigm.

Mike
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread Francesco Rabbi
Il giorno 28/mar/2011, alle ore 16:41, Mike Chase
mike.ch...@alternatemetaverse.com ha scritto:

 Is there a Linux build of V2 of any version that doesnt exhibit the
 annoying multi-second pauses that freeze the UI?  I find myself without
 any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both
 still have this issue.

 How in the world did this every get past QA?  It really renders the
 viewer unusable. I've been using Imprudence the last few days which is
 nice but a huge shift in UI and I've actually gotten both used to and
 productive with the V2 paradigm.

Can you explain better the kind of pause? I don't notice sort of...



-- 
Sent by iPhone
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread leliel
On Mon, Mar 28, 2011 at 7:50 AM, Francesco Rabbi syt...@gmail.com wrote:
 Il giorno 28/mar/2011, alle ore 16:41, Mike Chase
 mike.ch...@alternatemetaverse.com ha scritto:

 Is there a Linux build of V2 of any version that doesnt exhibit the
 annoying multi-second pauses that freeze the UI?  I find myself without
 any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both
 still have this issue.

http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/viewer-autobuild2010/latest.html

 How in the world did this every get past QA?  It really renders the
 viewer unusable. I've been using Imprudence the last few days which is
 nice but a huge shift in UI and I've actually gotten both used to and
 productive with the V2 paradigm.

I've been wondering that myself.

 Can you explain better the kind of pause? I don't notice sort of...

STORM-809
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread Discrete Dreamscape
It's due to libcurl, noted in STORM-809, and supposedly fixed (in the
autobuild repo)? If someone can verify that, maybe you can build it and
solve your problem, otherwise you'll have to build your own libcurl to drop
in. Another alternative seems to be maintaining your own DNS server/cache,
but that's pretty unnecessary IMO.

On Mon, Mar 28, 2011 at 10:50 AM, Francesco Rabbi syt...@gmail.com wrote:

 Il giorno 28/mar/2011, alle ore 16:41, Mike Chase
 mike.ch...@alternatemetaverse.com ha scritto:

  Is there a Linux build of V2 of any version that doesnt exhibit the
  annoying multi-second pauses that freeze the UI?  I find myself without
  any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both
  still have this issue.
 
  How in the world did this every get past QA?  It really renders the
  viewer unusable. I've been using Imprudence the last few days which is
  nice but a huge shift in UI and I've actually gotten both used to and
  productive with the V2 paradigm.

 Can you explain better the kind of pause? I don't notice sort of...



 --
 Sent by iPhone
 ___
 Policies and (un)subscribe information available here:
 http://wiki.secondlife.com/wiki/OpenSource-Dev
 Please read the policies before posting to keep unmoderated posting
 privileges

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread Boroondas Gupte
On 03/28/2011 04:41 PM, Mike Chase wrote:
 Is there a Linux build of V2 of any version that doesnt exhibit the 
 annoying multi-second pauses that freeze the UI?  I find myself without 
 any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both 
 still have this issue.
I guess you're seeing the effects of STORM-809
https://jira.secondlife.com/browse/STORM-809?

 How in the world did this every get past QA?  It really renders the 
 viewer unusable.
The effect of doing blocking domain name lookup is probably less
noticeable the faster the internet connection is. If you even have a
local nameserver which has most queried domains cached, you would hardly
see any difference. (Note that this was even mentioned
https://jira.secondlife.com/browse/STORM-809?focusedCommentId=241064page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-241064
as a workaround.)

So it could be that the QA department has such a caching nameserver, at
least somewhere in their LAN, and thus didn't notice this.

Boroondas
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread Opensource Obscure
On Mon, Mar 28, 2011 at 17:04, Boroondas Gupte
slli...@boroon.dasgupta.ch wrote:
 On 03/28/2011 04:41 PM, Mike Chase wrote:

 Is there a Linux build of V2 of any version that doesnt exhibit the
 annoying multi-second pauses that freeze the UI?  I find myself without
 any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both
 still have this issue.

I'm currently using the Autobuild development release which has been
mentioned above. Note that in this build some other important features
are broken, at least for me (I don't get any HTTP content). I still use it
because the performances degradation in 2.5 is too annoying to me.


 I guess you're seeing the effects of STORM-809?

 How in the world did this every get past QA?  It really renders the
 viewer unusable.

 The effect of doing blocking domain name lookup is probably less noticeable
 the faster the internet connection is. If you even have a local nameserver
 which has most queried domains cached, you would hardly see any difference.
 (Note that this was even mentioned as a workaround.)

I installed pdns but Second Life performances didn't improve much.
Geography may have a role here...I have a ~200ms ping time
(minimum) when connecting to Second Life servers. This is common
to all users connecting from Italy, as far as I can tell.


 So it could be that the QA department has such a caching nameserver, at
 least somewhere in their LAN, and thus didn't notice this.

Good point. I suggest they make sure they are NOT using such
a caching system, if it can hide issues as big as this one.

QA department apart, I / we should have done more testing.
I think I remember I had seen this, but I didn't bother to properly
investigate it / compare to other releases / file a report.

-- 
Opensource Obscure
http://twitter.com/oobscure - http://opensourceobscure.com/lol
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread leliel
On Mon, Mar 28, 2011 at 8:59 AM, Opensource Obscure
opensourceobsc...@gmail.com wrote:
 QA department apart, I / we should have done more testing.
 I think I remember I had seen this, but I didn't bother to properly
 investigate it / compare to other releases / file a report.

It was first reported on December 22, the same day the bug was
introduced. And was subsequently acknowledged by several Lindens in
late December and early January. Yet the bug still made it through 3
releases and counting. I don't think we as a community are the ones
that dropped the ball here.
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread Mike Chase
On 03/28/2011 10:53 AM, leliel wrote:
 On Mon, Mar 28, 2011 at 7:50 AM, Francesco Rabbisyt...@gmail.com  wrote:
 Il giorno 28/mar/2011, alle ore 16:41, Mike Chase
 mike.ch...@alternatemetaverse.com  ha scritto:

 Is there a Linux build of V2 of any version that doesnt exhibit the
 annoying multi-second pauses that freeze the UI?  I find myself without
 any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both
 still have this issue.
 http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/viewer-autobuild2010/latest.html

Is this a special build with the fix?  I hjave 2.6.3 which still 
exhibits the problem.  And yes I'd agree with others. I've seen this 
problem reported for quite some time yet no fix seems to make it into 
the released builds.

Mike

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread leliel
On Mon, Mar 28, 2011 at 9:16 AM, Mike Chase
mike.ch...@alternatemetaverse.com wrote:
 On 03/28/2011 10:53 AM, leliel wrote:

 On Mon, Mar 28, 2011 at 7:50 AM, Francesco Rabbisyt...@gmail.com  wrote:

 Il giorno 28/mar/2011, alle ore 16:41, Mike Chase
 mike.ch...@alternatemetaverse.com  ha scritto:

 Is there a Linux build of V2 of any version that doesnt exhibit the
 annoying multi-second pauses that freeze the UI?  I find myself without
 any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both
 still have this issue.


 http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/viewer-autobuild2010/latest.html

 Is this a special build with the fix?  I hjave 2.6.3 which still exhibits
 the problem.  And yes I'd agree with others. I've seen this problem reported
 for quite some time yet no fix seems to make it into the released builds.

It's the autobuild development branch which has several updated libs
including a fixed libcurl.
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Linux build without the pausing behaviour

2011-03-28 Thread Oz Linden (Scott Lawrence)
On 2011-03-28 10:53, leliel wrote:
 On Mon, Mar 28, 2011 at 7:50 AM, Francesco Rabbisyt...@gmail.com  wrote:
 Il giorno 28/mar/2011, alle ore 16:41, Mike Chase
 mike.ch...@alternatemetaverse.com  ha scritto:

 Is there a Linux build of V2 of any version that doesnt exhibit the
 annoying multi-second pauses that freeze the UI?  I find myself without
 any useable V2 viewer at present.  I've tried 2.5.2 and 2.6.3 and both
 still have this issue.
 http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/viewer-autobuild2010/latest.html


That branch is expected to merge back to viewer-development later this 
week, so the fix should be in the beta next week and the general release 
the week after next.


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges


Re: [opensource-dev] Review Request: (STORM-380) There is a little delay in sound when gesture first time played

2011-03-28 Thread Seth ProductEngine


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.h, lines 161-164
  http://codereview.secondlife.com/r/231/diff/1/?file=1326#file1326line161
 
  Please use tabs for indentation and spaces for alignment. (Here, that'd 
  be only one tab followed by 32 spaces.) Whatever you did here looks wrong 
  whether I set tab display length to 4 or 8.

Corrected indentation here and for the method declared above.


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 536-537
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line536
 
  If the iterator will not be used outside the loop, please declare it 
  with the assignment in the for's braces.
 
 Oz Linden wrote:
 I don't see any reason to make that distinction.

Seems like it may look confusing if the iterator is not used outside the loop, 
so moved its declaration inside braces.


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, line 549
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line549
 
  I'm not sure what you mean by if it stops. Maybe that the currently 
  handled gesture step stops the animation? If so, maybe write
  
  // Don't request the animation if this step stops it or if it is 
  already in Static VFS

Corrected.


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 582-587
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line582
 
  Why are STEP_CHAT and STEP_WAIT mentioned when they don't do anything 
  and we do have a default step that also doesn't do anything?
  
  If all cases are supposed to be explicitly handled, we might want to 
  make sure of that with:
  // [...]
  case STEP_CHAT:
  case STEP_WAIT:
  {
  break;
  }
  default:
  {
  // We should never get here.
  llassert(false);
  }
  }
  
  (... or maybe just some terminal output.)

Added the STEP_EOF for a complete list of step types and a warning for the 
default case.


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 763-764
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line763
 
  s/is/if/

Changed is: to if. Is that correct?


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 765-766
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line765
 
  Hmm ... so with the current change, a gesture's playback could 
  potentially be delayed forever due to assets for other gestures being 
  loaded?

For now starting each new gesture resets the list of pending assets downloads 
so if the last started gesture's asset are loaded successfully the playback 
will continue. This should probably need a better way to handle the cases of 
some assets not being loaded due to some kind of error. The current behavior is 
not suppressing the gesture playback due to any data loading delays. Should we 
follow this behavior?


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 1194-1197
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line1194
 
  Can a gesture reference other asset types than animations and sounds? 
  And if it can, shouldn't those be removed from mLoadingAssets, too, once 
  loaded?
  
  If it can but shouldn't maybe some terminal output would be appropriate 
  here.

It shouldn't reference other asset types and there are specific callback 
procedures to handle animation and sound assets so added an assert for the 
default case.


- Seth


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/231/#review506
---


On March 25, 2011, 5:33 p.m., Seth ProductEngine wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://codereview.secondlife.com/r/231/
 ---
 
 (Updated March 25, 2011, 5:33 p.m.)
 
 
 Review request for Viewer.
 
 
 Summary
 ---
 
 First pass implementation of syncing the animations and sounds before the 
 gesture starts playing.
 The actual playing of animations and sounds of a gesture starts only when all 
 needed animations and sound files are loaded into viewer cache. This reduces 
 the delay between animations and sounds meant to be played simultaneously but 
 may increase the delay between the moment a 

Re: [opensource-dev] Review Request: (STORM-380) There is a little delay in sound when gesture first time played

2011-03-28 Thread Seth ProductEngine


 On March 28, 2011, 4:06 a.m., Oz Linden wrote:
  indra/newview/llgesturemgr.cpp, lines 572-578
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line572
 
  Same possible race as above

Moved inserting the id into the mLoadingAssets prior to requesting asset data.

The getAssetData() request could potentially lead to memory leaks in some 
cases. Should this be a separate jira?


- Seth


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/231/#review508
---


On March 25, 2011, 5:33 p.m., Seth ProductEngine wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://codereview.secondlife.com/r/231/
 ---
 
 (Updated March 25, 2011, 5:33 p.m.)
 
 
 Review request for Viewer.
 
 
 Summary
 ---
 
 First pass implementation of syncing the animations and sounds before the 
 gesture starts playing.
 The actual playing of animations and sounds of a gesture starts only when all 
 needed animations and sound files are loaded into viewer cache. This reduces 
 the delay between animations and sounds meant to be played simultaneously but 
 may increase the delay between the moment a gesture is triggered and the 
 moment it starts playing.
 
 
 This addresses bug STORM-380.
 http://jira.secondlife.com/browse/STORM-380
 
 
 Diffs
 -
 
   indra/newview/llgesturemgr.h 6c15f820c3b9 
   indra/newview/llgesturemgr.cpp 6c15f820c3b9 
 
 Diff: http://codereview.secondlife.com/r/231/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Seth
 


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: (STORM-380) There is a little delay in sound when gesture first time played

2011-03-28 Thread Boroondas Gupte


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 763-764
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line763
 
  s/is/if/
 
 Seth ProductEngine wrote:
 Changed is: to if. Is that correct?

Correct.


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 765-766
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line765
 
  Hmm ... so with the current change, a gesture's playback could 
  potentially be delayed forever due to assets for other gestures being 
  loaded?
 
 Seth ProductEngine wrote:
 For now starting each new gesture resets the list of pending assets 
 downloads so if the last started gesture's asset are loaded successfully the 
 playback will continue. This should probably need a better way to handle the 
 cases of some assets not being loaded due to some kind of error. The current 
 behavior is not suppressing the gesture playback due to any data loading 
 delays. Should we follow this behavior?

Ah, I thought assets would only be removed from the list when they finished 
loading. If it were so, someone always triggering a new gesture (with new 
assets) before all previously queued assets have been loaded could thus prevent 
the list from becoming empty and thereby delay playback of all gestures 
forever, even although assets for most gestures would already be available. 
This could happen even in the case of no assets failing to load.

Emptying the list (is that line 532 in the new indra/newview/llgesturemgr.cpp 
?) when a new gesture comes in avoids this. Off course it isn't really the 
Right Thing To Do(TM), but if the behavior will in no case be worse than the 
current code, we can go for this until pending downloads are checked separately 
for each queued gesture. The latter can probably be achieved by maintaining a 
separate list of pending assets for each gesture and start playing a gesture 
when its own list has become empty.


 On March 26, 2011, 8:55 a.m., Boroondas Gupte wrote:
  indra/newview/llgesturemgr.cpp, lines 1194-1197
  http://codereview.secondlife.com/r/231/diff/1/?file=1327#file1327line1194
 
  Can a gesture reference other asset types than animations and sounds? 
  And if it can, shouldn't those be removed from mLoadingAssets, too, once 
  loaded?
  
  If it can but shouldn't maybe some terminal output would be appropriate 
  here.
 
 Seth ProductEngine wrote:
 It shouldn't reference other asset types and there are specific callback 
 procedures to handle animation and sound assets so added an assert for the 
 default case.

Is this already checked somewhere before here, or could a bogus gesture trigger 
that assert? If the latter, a clean-up step and some terminal output would be 
friendlier than an assert, as we don't want the ability to 'shoot down' a 
Viewer by providing invalid data from remote.


- Boroondas


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/231/#review506
---


On March 28, 2011, 12:21 p.m., Seth ProductEngine wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://codereview.secondlife.com/r/231/
 ---
 
 (Updated March 28, 2011, 12:21 p.m.)
 
 
 Review request for Viewer.
 
 
 Summary
 ---
 
 First pass implementation of syncing the animations and sounds before the 
 gesture starts playing.
 The actual playing of animations and sounds of a gesture starts only when all 
 needed animations and sound files are loaded into viewer cache. This reduces 
 the delay between animations and sounds meant to be played simultaneously but 
 may increase the delay between the moment a gesture is triggered and the 
 moment it starts playing.
 
 
 This addresses bug STORM-380.
 http://jira.secondlife.com/browse/STORM-380
 
 
 Diffs
 -
 
   indra/newview/llgesturemgr.h b3cfba00a29b 
   indra/newview/llgesturemgr.cpp b3cfba00a29b 
 
 Diff: http://codereview.secondlife.com/r/231/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Seth
 


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] Review Request: Enable CURLOPT_ENCODING for Inventory caps, which uses the LLURLRequest code path

2011-03-28 Thread Stone Linden

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/242/
---

Review request for Viewer, Oz Linden, Joshua Linden, and Brad Kittenbrink.


Summary
---

Enable Accept-Encoding: deflate, gzip in libcurl via setopt CURLOPT_ENCODING. 
I'm approaching this for Inventory, but it would apply to any HTTP request that 
goes through the LLURLRequest code path (vs. the LLCurl code path, which 
already does this).


Diffs
-

  indra/llmessage/llurlrequest.cpp 2ae060c0fa91 

Diff: http://codereview.secondlife.com/r/242/diff


Testing
---

Inventory loads, and I see the encoding options coming through on the backend 
apache logs.


Thanks,

Stone

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: Enable CURLOPT_ENCODING for Inventory caps, which uses the LLURLRequest code path

2011-03-28 Thread Monty Brandenberg

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/242/#review512
---


Before shipping, review the exploit history around CURLOPT_ENCODING.  There is a
known buffer overflow exploit, I believe in pre-7.20 releases but that should be
checked first for applicability.

- Monty


On March 28, 2011, 6:22 p.m., Stone Linden wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://codereview.secondlife.com/r/242/
 ---
 
 (Updated March 28, 2011, 6:22 p.m.)
 
 
 Review request for Viewer, Oz Linden, Joshua Linden, and Brad Kittenbrink.
 
 
 Summary
 ---
 
 Enable Accept-Encoding: deflate, gzip in libcurl via setopt CURLOPT_ENCODING. 
 I'm approaching this for Inventory, but it would apply to any HTTP request 
 that goes through the LLURLRequest code path (vs. the LLCurl code path, which 
 already does this).
 
 
 Diffs
 -
 
   indra/llmessage/llurlrequest.cpp 2ae060c0fa91 
 
 Diff: http://codereview.secondlife.com/r/242/diff
 
 
 Testing
 ---
 
 Inventory loads, and I see the encoding options coming through on the backend 
 apache logs.
 
 
 Thanks,
 
 Stone
 


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: VWR-20801 Implement SOCKS 5 Proxy for the viewer

2011-03-28 Thread Philippe (Merov) Bossut
Hi Robin,

Fantastic! I just went through the code and I'm not done reviewing and
putting my Ship it! stamp on it but I wanted to thank you for completing
this massive task. Thanks a lot!

Cheers,
- Merov

On Mon, Mar 28, 2011 at 4:46 AM, Robin Cornelius
robin.cornel...@gmail.comwrote:

This is an automatically generated e-mail. To reply, visit:
 http://codereview.secondlife.com/r/232/
   Review request for Viewer.
 By Robin Cornelius.
 Description

 VWR-20801 - Add ability to use SOCKS 5 proxy to the viewer. This allows the 
 UDP and/or the http requests to be sent via a SOCKS 5 proxy. This also allows 
 http proxies to be used for other http operations such as caps etc as 
 required. All the proxy settings have been unified on a single proxy floater 
 accessable from preferences.

   Testing

 Verified login and in world interaction with proxy disabled, verified login 
 and in world interactionvia socks 5 proxy. Code has been tested on Windows 
 very recently and has also worked fine on linux, but i'm not currently in a 
 position to retest that or Mac at all. Much more testing is needed to verify 
 this does not break anything unexpectedly and also works as expected when 
 enabled. To test requires a working socks 5 proxy.

   *Bugs: * VWR-20801 http://jira.secondlife.com/browse/VWR-20801
 Diffs

- indra/llmessage/CMakeLists.txt (65ff7415f171)
- indra/llmessage/llcurl.cpp (65ff7415f171)
- indra/llmessage/llpacketring.h (65ff7415f171)
- indra/llmessage/llpacketring.cpp (65ff7415f171)
- indra/llmessage/llsocks5.h (PRE-CREATION)
- indra/llmessage/llsocks5.cpp (PRE-CREATION)
- indra/llmessage/net.h (65ff7415f171)
- indra/llmessage/net.cpp (65ff7415f171)
- indra/newview/app_settings/settings.xml (65ff7415f171)
- indra/newview/llfloaterpreference.h (65ff7415f171)
- indra/newview/llfloaterpreference.cpp (65ff7415f171)
- indra/newview/llstartup.h (65ff7415f171)
- indra/newview/llstartup.cpp (65ff7415f171)
- indra/newview/llviewerfloaterreg.cpp (65ff7415f171)
- indra/newview/llxmlrpctransaction.cpp (65ff7415f171)
- indra/newview/skins/default/xui/en/floater_preferences_proxy.xml
(PRE-CREATION)
- indra/newview/skins/default/xui/en/notifications.xml (65ff7415f171)
- indra/newview/skins/default/xui/en/panel_preferences_setup.xml
(65ff7415f171)

 View Diff http://codereview.secondlife.com/r/232/diff/

 ___
 Policies and (un)subscribe information available here:
 http://wiki.secondlife.com/wiki/OpenSource-Dev
 Please read the policies before posting to keep unmoderated posting
 privileges

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] No IW Viewer Coding and Design meeting

2011-03-28 Thread Philippe (Merov) Bossut
Hi,

I've a conflicting internal meeting tomorrow right at the time of our
regular weekly so I need to cancel that this week.

We'll get together next week at the regular time.

Cheers,
- Merov
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges