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

Seth ProductEngine slitovchuk at productengine.com
Tue Mar 29 10:39:00 PDT 2011



> 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?
> 
> Boroondas Gupte wrote:
>     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.

Yes, it's line 532 but probably there should be a loading assets map that holds assets for each gesture.


> 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.
> 
> Boroondas Gupte wrote:
>     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.

That assert could only be triggered by requesting an asset type other that animation or sound with onAssetLoadComplete() callback, not by the invalid data from server. This callback can only handle those two types, for other types we don't know how to clean up a void pointer to user_data arg.


- Seth


-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110329/56513e0e/attachment-0001.htm 


More information about the opensource-dev mailing list