Skip to content
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

feat: updated codec versions #507

Conversation

Ouwen
Copy link
Contributor

@Ouwen Ouwen commented Jan 15, 2023

Updates codecs to current version.
Fixes this issue: #506

@netlify
Copy link

netlify bot commented Jan 15, 2023

Deploy Preview for cornerstone-wado-image-loader ready!

Name Link
🔨 Latest commit 5cd920e
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-wado-image-loader/deploys/63eac410af98100008f775e9
😎 Deploy Preview https://deploy-preview-507--cornerstone-wado-image-loader.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sedghi
Copy link
Member

sedghi commented Jan 16, 2023

So regarding the Charls decode in ms issue you mentioned in the cs3d. 50 iterations average

I run the test for the old vs new

Old:

Decode of ../fixtures/CT1.JLS took 11.707018479999999 ms
  frameInfo =  { width: 512, height: 512, bitsPerSample: 16, componentCount: 1 }
  decoded length =  524288
Decode of ../fixtures/CT2.JLS took 10.63982479 ms
  frameInfo =  { width: 512, height: 512, bitsPerSample: 16, componentCount: 1 }
  decoded length =  524288
Decode of ../../extern/charls/test/test.jls took 0.22356675 ms
  frameInfo =  { width: 4, height: 4, bitsPerSample: 8, componentCount: 1 }
  decoded length =  16
Encode of ../fixtures/CT2.RAW took 135.857398 ms
  encoded length= 115504

New:

Decode of ../fixtures/CT1.JLS took 19.61669452 ms
  frameInfo =  { width: 512, height: 512, bitsPerSample: 16, componentCount: 1 }
  decoded length =  524288
Decode of ../fixtures/CT2.JLS took 14.439553929999999 ms
  frameInfo =  { width: 512, height: 512, bitsPerSample: 16, componentCount: 1 }
  decoded length =  524288
Decode of ../../extern/charls/test/test.jls took 0.53320259 ms
  frameInfo =  { width: 4, height: 4, bitsPerSample: 8, componentCount: 1 }
  decoded length =  16
Encode of ../fixtures/CT2.RAW took 14.74438806 ms
  encoded length= 115504

I guess it is not significantly worse. But the encode is significantly better

@Ouwen
Copy link
Contributor Author

Ouwen commented Jan 16, 2023

hmmm that's really interesting that the decode is not slower... I'll try to benchmark and post a video of what I'm seeing (maybe it's a M2 issue?)

I don't see any diffs when I use safari or mobile chrome fwiw

@Ouwen Ouwen force-pushed the gradienthealth/updated_codec_versions branch from cfed2a0 to 89fb2f4 Compare February 1, 2023 00:05
@Ouwen
Copy link
Contributor Author

Ouwen commented Feb 13, 2023

@sedghi ready for review!

@sedghi
Copy link
Member

sedghi commented Feb 14, 2023

Just to add to the comments above, we did rigorous testings and this PR does not add performance issues anymore

@sedghi sedghi merged commit 0d3eabf into cornerstonejs:master Feb 14, 2023
@sedghi
Copy link
Member

sedghi commented Feb 14, 2023

🎉 This PR is included in version 4.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants