# HG changeset patch # User Alain Mazy # Date 1715848702 -7200 # Node ID a8408ef2b2d87b07d2dfe9f8a28d68b2f3072ced # Parent ef4a0f6d977782277f07f1fd0de6c98a259436dd# Parent a6b4e0abe532f17d86d9ae0f9ce57f4437b6037a merge cmove2 -> default diff -r ef4a0f6d9777 -r a8408ef2b2d8 NEWS --- a/NEWS Sat Apr 06 17:28:00 2024 +0200 +++ b/NEWS Thu May 16 10:38:22 2024 +0200 @@ -2,6 +2,13 @@ =============================== * Fix signature of "orthanc.RestOutput.SendHttpStatus()" +* Added orthanc.RegisterMoveCallback2() that takes 4 callbacks like the + original C SDK function. This allows you to implement a correct handling + of the C-Move sub-operations count in the GetMoveSize(). The ApplyMove() + must now handle a single sub-operation at a time. + The legacy orthanc.RegisterMoveCallback() always considers that there is a single + sub-operation and we have observed modalities complaining that the number of + sub-operations was not matching the number of instances sent. Version 4.1 (2023-08-30) diff -r ef4a0f6d9777 -r a8408ef2b2d8 Sources/DicomScpCallbacks.cpp --- a/Sources/DicomScpCallbacks.cpp Sat Apr 06 17:28:00 2024 +0200 +++ b/Sources/DicomScpCallbacks.cpp Thu May 16 10:38:22 2024 +0200 @@ -30,6 +30,12 @@ static PyObject* moveScpCallback_ = NULL; static PyObject* worklistScpCallback_ = NULL; +// version 2 of Move callbacks +static PyObject* createMoveScpDriverCallback_ = NULL; +static PyObject* getMoveSizeCallback_ = NULL; +static PyObject* applyMoveCallback_ = NULL; +static PyObject* freeMoveCallback_ = NULL; + static PyObject *GetFindQueryTag(sdk_OrthancPluginFindQuery_Object* self, PyObject *args, @@ -399,6 +405,226 @@ } +static void* CreateMoveCallback2(OrthancPluginResourceType resourceType, + const char *patientId, + const char *accessionNumber, + const char *studyInstanceUid, + const char *seriesInstanceUid, + const char *sopInstanceUid, + const char *originatorAet, + const char *sourceAet, + const char *targetAet, + uint16_t originatorId) +{ + assert(createMoveScpDriverCallback_ != NULL); + + try + { + std::string _patientId, _accessionNumber, _studyInstanceUid, _seriesInstanceUid, _sopInstanceUid, _originatorAet, _sourceAet, _targetAet; + if (patientId != NULL) + { + _patientId.assign(patientId); + } + + if (accessionNumber != NULL) + { + _accessionNumber.assign(accessionNumber); + } + + if (studyInstanceUid != NULL) + { + _studyInstanceUid.assign(studyInstanceUid); + } + + if (seriesInstanceUid != NULL) + { + _seriesInstanceUid.assign(seriesInstanceUid); + } + + if (sopInstanceUid != NULL) + { + _sopInstanceUid.assign(sopInstanceUid); + } + + if (originatorAet != NULL) + { + _originatorAet.assign(originatorAet); + } + + if (sourceAet != NULL) + { + _sourceAet.assign(sourceAet); + } + + if (targetAet != NULL) + { + _targetAet.assign(targetAet); + } + + PythonLock lock; + + PythonObject kw(lock, PyDict_New()); + + std::string level; + switch (resourceType) + { + case OrthancPluginResourceType_Patient: + level = "PATIENT"; + break; + + case OrthancPluginResourceType_Study: + level = "STUDY"; + break; + + case OrthancPluginResourceType_Series: + level = "SERIES"; + break; + + case OrthancPluginResourceType_Instance: + level = "INSTANCE"; + break; + + default: + throw OrthancPlugins::PluginException(OrthancPluginErrorCode_ParameterOutOfRange); + } + + { + PythonString tmp(lock, level); + PyDict_SetItemString(kw.GetPyObject(), "Level", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _patientId); + PyDict_SetItemString(kw.GetPyObject(), "PatientID", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _accessionNumber); + PyDict_SetItemString(kw.GetPyObject(), "AccessionNumber", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _studyInstanceUid); + PyDict_SetItemString(kw.GetPyObject(), "StudyInstanceUID", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _seriesInstanceUid); + PyDict_SetItemString(kw.GetPyObject(), "SeriesInstanceUID", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _sopInstanceUid); + PyDict_SetItemString(kw.GetPyObject(), "SOPInstanceUID", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _originatorAet); + PyDict_SetItemString(kw.GetPyObject(), "OriginatorAET", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _sourceAet); + PyDict_SetItemString(kw.GetPyObject(), "SourceAET", tmp.GetPyObject()); + } + + { + PythonString tmp(lock, _targetAet); + PyDict_SetItemString(kw.GetPyObject(), "TargetAET", tmp.GetPyObject()); + } + + { + PythonObject tmp(lock, PyLong_FromUnsignedLong(originatorId)); + PyDict_SetItemString(kw.GetPyObject(), "OriginatorID", tmp.GetPyObject()); + } + + PythonObject args(lock, PyTuple_New(0)); + + // Note: the result is not attached to the PythonLock because we want it to survive after this call since + // the result is the python move driver that will be passed as first argument to GetMoveSize, Apply and Free. + // After the PyObject_Call, result's ref count is 1 -> no need to add a reference but we need to decref explicitely + // to delete the object at the end of the move. + PyObject* result = PyObject_Call(createMoveScpDriverCallback_, args.GetPyObject(), kw.GetPyObject()); + + OrthancPluginErrorCode code = lock.CheckCallbackSuccess("Python C-MOVE SCP callback (Create)"); + if (code != OrthancPluginErrorCode_Success) + { + throw OrthancPlugins::PluginException(code); + } + + return result; + } + catch (OrthancPlugins::PluginException& e) + { + return NULL; + } +} + + +static uint32_t GetMoveSize2(void *moveDriver) +{ + assert(moveDriver != NULL); + assert(getMoveSizeCallback_ != NULL); + + PythonLock lock; + + PythonObject args(lock, PyTuple_New(1)); + PyTuple_SetItem(args.GetPyObject(), 0, reinterpret_cast(moveDriver)); + Py_INCREF(moveDriver); // because PyTuple_SetItem steals a reference and we need to keep the object alive + + PythonObject result(lock, PyObject_CallObject(getMoveSizeCallback_, args.GetPyObject())); + + OrthancPluginErrorCode code = lock.CheckCallbackSuccess("Python C-MOVE SCP callback (GetMoveSize)"); + if (code != OrthancPluginErrorCode_Success) + { + throw OrthancPlugins::PluginException(code); + } + + if (!PyLong_Check(result.GetPyObject())) + { + throw OrthancPlugins::PluginException(OrthancPluginErrorCode_BadParameterType); + } + + return static_cast(PyLong_AsLong(result.GetPyObject())); +} + + +OrthancPluginErrorCode ApplyMove2(void *moveDriver) +{ + assert(moveDriver != NULL); + assert(applyMoveCallback_ != NULL); + + PythonLock lock; + + PythonObject args(lock, PyTuple_New(1)); + PyTuple_SetItem(args.GetPyObject(), 0, reinterpret_cast(moveDriver)); + Py_INCREF(moveDriver); // because PyTuple_SetItem steals a reference and we need to keep the object alive + + PythonObject result(lock, PyObject_CallObject(applyMoveCallback_, args.GetPyObject())); + + OrthancPluginErrorCode code = lock.CheckCallbackSuccess("Python C-MOVE SCP callback (Apply)"); + return code; +} + + +void FreeMove2(void *moveDriver) +{ + assert(moveDriver != NULL); + + PythonLock lock; + + PythonObject args(lock, PyTuple_New(1)); + PyTuple_SetItem(args.GetPyObject(), 0, reinterpret_cast(moveDriver)); + Py_INCREF(moveDriver); // because PyTuple_SetItem steals a reference and we need to keep the object alive + + assert(freeMoveCallback_ != NULL); + PythonObject result(lock, PyObject_CallObject(freeMoveCallback_, args.GetPyObject())); + + lock.CheckCallbackSuccess("Python C-MOVE SCP callback (Free)"); + + Py_DECREF(moveDriver); // remove the initial reference -> this will delete the Python Object when we exit this function +} + OrthancPluginErrorCode WorklistCallback(OrthancPluginWorklistAnswers *answers, const OrthancPluginWorklistQuery *query, @@ -487,6 +713,25 @@ } +PyObject* RegisterMoveCallback2(PyObject* module, PyObject* args) +{ + // The GIL is locked at this point (no need to create "PythonLock") + + class Registration : public ICallbackRegistration + { + public: + virtual void Register() ORTHANC_OVERRIDE + { + OrthancPluginRegisterMoveCallback( + OrthancPlugins::GetGlobalContext(), CreateMoveCallback2, GetMoveSize2, ApplyMove2, FreeMove2); + } + }; + + Registration registration; + return ICallbackRegistration::Apply4( + registration, args, createMoveScpDriverCallback_, getMoveSizeCallback_, applyMoveCallback_, freeMoveCallback_, "Python C-MOVE SCP callback (2)"); +} + PyObject* RegisterWorklistCallback(PyObject* module, PyObject* args) { // The GIL is locked at this point (no need to create "PythonLock") @@ -511,4 +756,9 @@ ICallbackRegistration::Unregister(findScpCallback_); ICallbackRegistration::Unregister(moveScpCallback_); ICallbackRegistration::Unregister(worklistScpCallback_); + + ICallbackRegistration::Unregister(createMoveScpDriverCallback_); + ICallbackRegistration::Unregister(getMoveSizeCallback_); + ICallbackRegistration::Unregister(applyMoveCallback_); + ICallbackRegistration::Unregister(freeMoveCallback_); } diff -r ef4a0f6d9777 -r a8408ef2b2d8 Sources/DicomScpCallbacks.h --- a/Sources/DicomScpCallbacks.h Sat Apr 06 17:28:00 2024 +0200 +++ b/Sources/DicomScpCallbacks.h Thu May 16 10:38:22 2024 +0200 @@ -26,6 +26,8 @@ PyObject* RegisterMoveCallback(PyObject* module, PyObject* args); +PyObject* RegisterMoveCallback2(PyObject* module, PyObject* args); + PyObject* RegisterWorklistCallback(PyObject* module, PyObject* args); void FinalizeDicomScpCallbacks(); diff -r ef4a0f6d9777 -r a8408ef2b2d8 Sources/ICallbackRegistration.cpp --- a/Sources/ICallbackRegistration.cpp Sat Apr 06 17:28:00 2024 +0200 +++ b/Sources/ICallbackRegistration.cpp Thu May 16 10:38:22 2024 +0200 @@ -97,6 +97,55 @@ } } +PyObject *ICallbackRegistration::Apply4(ICallbackRegistration& registration, + PyObject* args, + PyObject*& singletonCallback1, + PyObject*& singletonCallback2, + PyObject*& singletonCallback3, + PyObject*& singletonCallback4, + const std::string& details) +{ + // https://docs.python.org/3/extending/extending.html#calling-python-functions-from-c + PyObject* callback1 = NULL; + PyObject* callback2 = NULL; + PyObject* callback3 = NULL; + PyObject* callback4 = NULL; + + if (!PyArg_ParseTuple(args, "OOOO", &callback1, &callback2, &callback3, &callback4) || + callback1 == NULL || callback2 == NULL || callback3 == NULL || callback4 == NULL) + { + const std::string message = "Expected 4 callback functions to register " + details; + PyErr_SetString(PyExc_ValueError, message.c_str()); + return NULL; + } + else if (singletonCallback1 != NULL || singletonCallback2 != NULL || singletonCallback3 != NULL || singletonCallback4 != NULL) + { + const std::string message = "Can only register once for " + details; + PyErr_SetString(PyExc_RuntimeError, message.c_str()); + return NULL; + } + else + { + OrthancPlugins::LogInfo("Registering callbacks " + details); + registration.Register(); + + singletonCallback1 = callback1; + Py_XINCREF(singletonCallback1); + + singletonCallback2 = callback2; + Py_XINCREF(singletonCallback2); + + singletonCallback3 = callback3; + Py_XINCREF(singletonCallback3); + + singletonCallback4 = callback4; + Py_XINCREF(singletonCallback4); + + Py_INCREF(Py_None); + return Py_None; + } +} + void ICallbackRegistration::Unregister(PyObject*& singletonCallback) { diff -r ef4a0f6d9777 -r a8408ef2b2d8 Sources/ICallbackRegistration.h --- a/Sources/ICallbackRegistration.h Sat Apr 06 17:28:00 2024 +0200 +++ b/Sources/ICallbackRegistration.h Thu May 16 10:38:22 2024 +0200 @@ -47,5 +47,14 @@ PyObject*& singletonCallback2, const std::string& details); + // The GIL must be locked + static PyObject *Apply4(ICallbackRegistration& registration, + PyObject* args, + PyObject*& singletonCallback1, + PyObject*& singletonCallback2, + PyObject*& singletonCallback3, + PyObject*& singletonCallback4, + const std::string& details); + static void Unregister(PyObject*& singletonCallback); }; diff -r ef4a0f6d9777 -r a8408ef2b2d8 Sources/Plugin.cpp --- a/Sources/Plugin.cpp Sat Apr 06 17:28:00 2024 +0200 +++ b/Sources/Plugin.cpp Thu May 16 10:38:22 2024 +0200 @@ -384,7 +384,12 @@ PyMethodDef f = { "RegisterMoveCallback", RegisterMoveCallback, METH_VARARGS, "" }; functions.push_back(f); } - + + { + PyMethodDef f = { "RegisterMoveCallback2", RegisterMoveCallback2, METH_VARARGS, "" }; + functions.push_back(f); + } + { PyMethodDef f = { "RegisterWorklistCallback", RegisterWorklistCallback, METH_VARARGS, "" }; functions.push_back(f);