cornerstonejs / cornerstoneWADOImageLoader

[DEPRECATED] DICOM WADO Image Loader for the cornerstone library
MIT License
284 stars 266 forks source link

feat: updated codec versions #507

Closed Ouwen closed 1 year ago

Ouwen commented 1 year ago

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

netlify[bot] commented 1 year ago

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

Name Link
Latest commit 5cd920e6768f5bae477cd6f273416b3be015468d
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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@sedghi ready for review!

sedghi commented 1 year ago

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

sedghi commented 1 year ago

:tada: This PR is included in version 4.9.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: