New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix concurrent resize/read from summedBlendshapeCoefficients in RC62 #12117

Closed
wants to merge 1 commit into
base: RC62
from

Conversation

Projects
None yet
6 participants
@birarda
Member

birarda commented Jan 9, 2018

Candidate bug fix adding to upcoming release RC62 for at least one of the recent P1 avatar mixer crashes

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 9, 2018

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 9, 2018

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 9, 2018

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 9, 2018

@hifi-gustavo

This comment has been minimized.

Show comment
Hide comment

@ZappoMan ZappoMan changed the title from RC62 Hotfix: fix concurrent resize/read from summedBlendshapeCoefficients to fix concurrent resize/read from summedBlendshapeCoefficients in RC62 Jan 9, 2018

@ZappoMan ZappoMan added this to the RC62 milestone Jan 9, 2018

@ZappoMan

This comment has been minimized.

Show comment
Hide comment
@ZappoMan

ZappoMan Jan 9, 2018

Contributor

Note to: @conklin94122 @RyanDowne - I believe this fix should be included in RC62... but it's fixing a very old bug, that we only recently noticed in very specific contexts.

Test Plan:

  • this code changes the low level storage for the avatar "head" data... this means we need to run all tests related to avatars and their head/facial data.
  • run a large crowd test
  • run a lot of recording play back with facial animation
  • run any sample code that messes with blendshapes
  • run any tests related to facial capture
  • make sure to test on a server that has > 16 cores and is unix
Contributor

ZappoMan commented Jan 9, 2018

Note to: @conklin94122 @RyanDowne - I believe this fix should be included in RC62... but it's fixing a very old bug, that we only recently noticed in very specific contexts.

Test Plan:

  • this code changes the low level storage for the avatar "head" data... this means we need to run all tests related to avatars and their head/facial data.
  • run a large crowd test
  • run a lot of recording play back with facial animation
  • run any sample code that messes with blendshapes
  • run any tests related to facial capture
  • make sure to test on a server that has > 16 cores and is unix
@sbennett77

This comment has been minimized.

Show comment
Hide comment
@sbennett77

sbennett77 Jan 9, 2018

Currently Testing.

sbennett77 commented Jan 9, 2018

Currently Testing.

@conklin94122

This comment has been minimized.

Show comment
Hide comment
@conklin94122

conklin94122 Jan 9, 2018

Contributor

@ZappoMan Would 20 bots and 5 testers work for a large crowd, or do we need more people and/or bots?

Contributor

conklin94122 commented Jan 9, 2018

@ZappoMan Would 20 bots and 5 testers work for a large crowd, or do we need more people and/or bots?

@birarda

This comment has been minimized.

Show comment
Hide comment
@birarda

birarda Jan 9, 2018

Member

Closing this since it doesn't fix the P1 we're focused on for RC62. I'll re-open a PR against 63 (with some additional changes to make getMyHead and getHead return references) shortly.

Member

birarda commented Jan 9, 2018

Closing this since it doesn't fix the P1 we're focused on for RC62. I'll re-open a PR against 63 (with some additional changes to make getMyHead and getHead return references) shortly.

@birarda birarda closed this Jan 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment