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

CSTORE Association before sending the request because of multiple presentation contexts #1672

Closed
jakubsuchybio opened this issue Nov 1, 2023 · 7 comments
Labels

Comments

@jakubsuchybio
Copy link

jakubsuchybio commented Nov 1, 2023

Hi, this is just a question not a bug or a feature request.

We are in the process of migrating from dicom-cs (like 10years old) library to the fo-dicom.
But we hit a wall a bit with CSTORE.

Because our dicom client is able to send CSTORE in multiple presentation contexts (encapsulated pdf, secondary capture image, multi frame image, twelve lead ecg)

In the old dicom-cs we were doing the association RQ first with multiple presentation contexts, then we would get the association response and after that we would construct the CSTORE request in the accepted presentation context and then send the CSTORE request.

However in fo-dicom In the example from https://saravanansubramanian.com/dicom/ there is just one presentation context, so there was no need to do the assocRQ beforehand, which we need.

I already tried asking ChatGPT if it would help: https://chat.openai.com/share/bef1bc8a-376b-46e4-9272-ce192c335402
And it says to do this sequence:

DicomClient client = new DicomClient();
bool associationAccepted = false;
// Add your potential presentation contexts
client.AdditionalPresentationContexts.Add(...);
client.AssociationAccepted += (sender, e) =>
{
    associationAccepted = true;
    // Check accepted presentation contexts and prepare your C-STORE request
    // ...
};
await client.SendAsync("127.0.0.1", 104, false, "SCU", "SCP");
if (associationAccepted)
{
    // Now that the association has been accepted, create and send your C-STORE request
    var cstoreRequest = new DicomCStoreRequest(yourDicomFile);
    client.AddRequest(cstoreRequest);
    await client.SendAsync("127.0.0.1", 104, false, "SCU", "SCP");
}

But I don't know, if we can SendAsync without any request in the queue, only with the PresentationContexts added.
If not, can you please point us to the direction that would work for us?

Thank you very much for any response.

Edit:
Also it suggest one other way with

DicomClient client = new DicomClient();

// Add multiple presentation contexts that you're willing to accept
client.AdditionalPresentationContexts.Add(DicomPresentationContext.GetScpRoleTransferSyntaxes(DicomUID.CTImageStorage));
client.AdditionalPresentationContexts.Add(DicomPresentationContext.GetScpRoleTransferSyntaxes(DicomUID.MRImageStorage));
// ... add more as needed

client.AssociationAccepted += (sender, e) =>
{
    // Check which presentation context(s) were accepted
    foreach (var pc in e.Association.PresentationContexts)
    {
        if (pc.Result == DicomPresentationContextResult.Accept)
        {
            // Create and send C-STORE request with this specific presentation context
            var cstoreRequest = new DicomCStoreRequest(yourDicomFile);
            // ... set details for cstoreRequest
            
            client.AddRequest(cstoreRequest);
            break;  // Assuming you'll use the first accepted context
        }
    }
};

await client.SendAsync("127.0.0.1", 104, false, "YOUR_AE_TITLE", "REMOTE_AE_TITLE");

But here the flow is kinda weird...
At first we are starting to send via SendAsync just the presentationContexts, then it jumps to the AssocAccepted event handler to AddRequest to the accepted PresContext and then it finishes the SendAsync.

I don't know if any of these ways would work. 😅

@amoerie
Copy link
Collaborator

amoerie commented Nov 2, 2023

Hi @jakubsuchybio

First, you should be aware of the fact that fo-dicom provides 2 ways to send DICOM requests:

  1. DicomClient
  2. AdvancedDicomClientConnectionFactory

DicomClient is pretty high level and handles the DICOM specific details for you, such as association request and release handling. This is done automatically based on the DICOM requests that you enqueue with the DicomClient.

That is why the API looks like this:

var request = new DicomCEchoRequest
{
    OnResponseReceived = (req, res) => { ... }
    OnTimeout = (sender, args) => { ... }
};
var client = DicomClientFactory.Create("127.0.0.1", port, false, "SCU", "ANY-SCP");
await client.AddRequestAsync(request);
await client.SendAsync();

Here, the DICOM client will automatically add the necessary SOP Class UIDs for the C-ECHO request in the association request.
So, in your case, you can keep using DicomClient and do the following:

List<DicomRequest> requests = ...; // prepare your DICOM requests in advance
var client = DicomClientFactory.Create("127.0.0.1", port, false, "SCU", "ANY-SCP");
foreach(var request in requests) 
{
    await client.AddRequestAsync(request);
}
await client.SendAsync();

If you don't want to create all of your requests in advance, but you do know somehow which abstract syntaxes + transfer syntaxes will be needed, you can also do this:

var presentationContexts = new DicomPresentationContextCollection();
presentationContexts.Add(DicomUID.EncapsulatedPDFStorage, DicomTransferSyntax.ImplicitVRLittleEndian, DicomTransferSyntax.ExplicitVRLittleEndian);
presentationContexts.Add(DicomUID.SecondaryCaptureImageStorage, DicomTransferSyntax.ExplicitVRLittleEndian);
foreach (var pc in presentationContexts)
{
    client.AdditionalPresentationContexts.Add(pc);
}

However, the fact remains that DicomClient will only open a DICOM association when you actually send a request, and DicomClient does not offer control over when to open and close an association. It does this internally and automatically.

If you require full control over the inner DICOM handshake, you'll need to use AdvancedDicomClientConnectionFactory. (Note that DicomClient uses this internally, so you will have all of the same possibilities)

This is how you send a C-ECHO with the advanced API (copied from the README):

var cancellationToken = CancellationToken.None;
var connectionRequest = new AdvancedDicomClientConnectionRequest
{
    NetworkStreamCreationOptions = new NetworkStreamCreationOptions
    {
        Host = "127.0.0.1",
        Port = server.Port,
    }
};

// Alternatively, inject IAdvancedDicomClientConnectionFactory via dependency injection instead of using this static method
using var connection = await AdvancedDicomClientConnectionFactory.OpenConnectionAsync(connectionRequest, cancellationToken);

var associationRequest = new AdvancedDicomClientAssociationRequest
{
    CallingAE = "EchoSCU",
    CalledAE = "EchoSCP",
    // Optionally negotiate user identity
    UserIdentityNegotiation = new DicomUserIdentityNegotiation
    {
        UserIdentityType = DicomUserIdentityType.UsernameAndPasscode,
        PositiveResponseRequested = true,
        PrimaryField = "USERNAME",
        SecondaryField = "PASSCODE",
    }
};

var cEchoRequest = new DicomCEchoRequest();

associationRequest.PresentationContexts.AddFromRequest(cEchoRequest);
associationRequest.ExtendedNegotiations.AddFromRequest(cEchoRequest);

using var association = await connection.OpenAssociationAsync(associationRequest, cancellationToken);
try
{
    DicomCEchoResponse cEchoResponse = await association.SendCEchoRequestAsync(cEchoRequest, cancellationToken).ConfigureAwait(false);
    
    Console.WriteLine(cEchoResponse.Status);
}
finally
{
    await association.ReleaseAsync(cancellationToken);
}

Using AdvancedDicomClientAssociationRequest.PresentationContexts you will have full control over the presentation contexts.

Does this answer your question?

@jakubsuchybio
Copy link
Author

Thanks for the very informative answer.
Didn't know about the AdvancedDicomClientConnectionRequest, that would indeed be a solution for us.

But our scenario is a bit different.
We are able to send different formats of CSTORE (those 4) and we ask the server in which format they want it.
And then only construct 1 of those 4 to send. So that we don't waste resources creating 4 requests, when 3 would not be sent.

So I guess DicomClient is unusable for us in this particular scenario (association first, then send request based on the association result).

I'm just thinking, if DicomClient could be somehow enhanced to also support this scenario or if it would get only bloated with some advanced stuff that is not needed there.

@amoerie
Copy link
Collaborator

amoerie commented Nov 2, 2023

@jakubsuchybio If I understand correctly, you are saying that your application keeps the DICOM files in multiple formats already, and you want to send the DICOM file immediately in the format that the server requests it to avoid redundant transcoding?

You could probably get this working by sending a dummy ECHO request, capture the accepted association and then use that to add more request to the DicomClient while it's already sending.
Something like this:

var client = DicomClientFactory.Create("127.0.0.1", port, false, "SCU", "ANY-SCP");

// Use this setting to keep DICOM associations open for a little while after the last request has completed
// If you add more requests to the same client, it will reuse the existing association rather than opening a new one each time
// Just be aware that this will also prolong the SendAsync call after all requests have completed
client.ClientOptions.AssociationLingerTimeoutInMs = (int) TimeSpan.FromMinutes(1).TotalMilliseconds;

// Setup your presentation contexts
var presentationContexts = new DicomPresentationContextCollection();
presentationContexts.Add(DicomUID.EncapsulatedPDFStorage, DicomTransferSyntax.ImplicitVRLittleEndian, DicomTransferSyntax.ExplicitVRLittleEndian);
presentationContexts.Add(DicomUID.SecondaryCaptureImageStorage, DicomTransferSyntax.ExplicitVRLittleEndian);
foreach (var pc in presentationContexts)
{
    client.AdditionalPresentationContexts.Add(pc);
}

// Send an initial echo request and capture the association response in a TaskCompletionSource
var associationTask = new TaskCompletionSource<DicomAssociation>(TaskCreationOptions.RunContinuationsAsynchronously);
client.AssociationAccepted += (_, associationAcceptedEventArgs) =>
{
    associationTask.TrySetResult(associationAcceptedEventArgs.Association);
};

var echoRequest = new DicomCEchoRequest();
await client.AddRequestAsync(echoRequest);

// Start sending, but don't await
var sendTask = client.SendAsync();

// Wait until the association is established
var association = await associationTask.Task;

// Now we can add C-STORE requests based on the last association
var encapsulatedPdfStorageTransferSyntax = association.PresentationContexts
    .Where(pc => pc.AbstractSyntax == DicomUID.EncapsulatedPDFStorage)
    .Select(pc => pc.AcceptedTransferSyntax)
    .First();

DicomCStoreRequest cStoreRequest;
if (encapsulatedPdfStorageTransferSyntax == DicomTransferSyntax.ExplicitVRLittleEndian)
{
    cStoreRequest = new DicomCStoreRequest("pdf.explicit.dcm");
}
else if (encapsulatedPdfStorageTransferSyntax == DicomTransferSyntax.ImplicitVRLittleEndian)
{
    cStoreRequest = new DicomCStoreRequest("pdf.implicit.dcm");
}
else
{
    throw new NotImplementedException();
}

await client.AddRequestAsync(cStoreRequest);

if (client.IsSendRequired)
{
    // we took a little long, client already stopped sending
    // await initial send task to expose exceptions or cancellation
    await sendTask;
    sendTask = client.SendAsync();
}

await sendTask;

But, I'll be honest, I'm not sure I would recommend this. The advanced API seems better suited to your needs.
Or, like you said, we could add built in support for this use case in fo-dicom itself. I imagine it would be possible then to create a single DicomCStoreRequest with multiple DICOM files, one for each precomputed transfer syntax.
Then, after the association handshake, we could check if the accepted transfer syntax is already precomputed.
However, if it isn't, which file do you start from then to transcode?
I don't think this would be very simple to implement.

@jakubsuchybio
Copy link
Author

jakubsuchybio commented Nov 2, 2023

Yeah, we figured that with dummy echo, we could get the accepted association and with second request we would send the CSTORE.

And no, we don't have multiple formats beforehand.
We have basically an examination in our prorietary format (ECG examination) and then pdf report for that examination.

Then we have a priority list of presentation contexts.
e.g. 1. pdf, 2. secondary, 3. multiframe 4. twelvelead

And only after we get some acceptance
e.g. multiframe
only then we take our PDF and rerender it into images and create CSTORE dicom request from those images.
But if we get the acceptance for the 1. pdf, then we don't rerender anything and send it as a pdf.

Thats what I'm talking about not to waste resources on rerendering into images or into twelve lead signal, if there is no need for it (if server accepts pdfs)

So we will try with the dummy echo on some of our Beta Testers and if it doesn't work, we will use that AdvancedClient.

The problem with Dicom Standard is, that everybody bends it to their will and a LOT of implementations doesn't conform to the standard at all. We see it day after day. Because of this we are rearchitecting our Dicom implementation from dicom-cs to fo-dicom, so that we have in the middle the xslt transformation from our models to dicom models and when someone is not conforming to the standard, we just bend our xslt transformation for them to work. Without the need for us to change the code and rebuild our application everytime someone doesn't conform to the standart :)

@amoerie
Copy link
Collaborator

amoerie commented Nov 2, 2023

Some cool ideas in your story!

I do wonder though: do these systems change their acceptance often? Wouldn't it suffice to just query the preferences once per system (AE title), and then just assume that for future associations. You could even automatically trigger a background recalibration if the preferences do change at some point.
But in my experience, PACS systems are fairly consistent in their inconsistency. 🙂

Anyway, I hope Fellow Oak Dicom is useful to you!

@gofal
Copy link
Contributor

gofal commented Nov 3, 2023

Can confirm from my experience that there are applications that just do the described method. I have seen entries on the SCP log, that there are echos with lots of abstractsyntaxes and various transfersyntaxes. So the client just wanted to check the features. Not only Storage SOP Classes, but this way you also can "ask" if C-Find, C-Get or C-Move, or Storage Commitment is accepted or rejected.

I also doubt that it woud be efficient to do this asking on every connection. Instead integrate this into your setup or configuration environment and then cache/store this setting.

And it was very interesting to see that you tried to find an answer for a very special usecase with chatGPT. Because it is not a common problem, chatGPT will not have found enough sample code to be trained with. I wonder if you asked some other fo-dicom related questions and chatGPT was able to generade working code?

@amoerie
Copy link
Collaborator

amoerie commented Nov 3, 2023

I can attest from personal experience that any Fellow Oak Dicom related questions posed to ChatGPT have been nothing but total and utter disappointments. It's probably like you say @gofal the dataset that is available in the wild is too thin.
It hallucinates APIs all the time.

@gofal gofal closed this as completed Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants