Conversation
…r); extract CapturesUserNotices test trait
Introduce a first-class video capability the multiframe_to_video shim will build on: - FrameTiming: named pacing intent (framesPerSecond, secondsPerFrame, repeatEachFrame frame-multiplier) that computes ffmpeg input/output rates. - VideoFormat: container/codec option-object; mp4() (H.264 + yuv420p) for now, shaped so other formats slot in without an API change. Carries an explicit muxer so output never depends on the path extension. - VideoEncoder: ffmpeg abstraction with its own PATH locator (Toolkit pins to a directory and does not search PATH); fail-loud ConversionException on a missing or failing ffmpeg, never a silent no-op. - Convert::toVideo(out, ?FrameTiming, ?VideoFormat, ?Windowing, ?Scale, quality): renders frames to a private temp dir (reusing toJpegFrames), assembles via VideoEncoder, always cleans up, never chdir. Defaults to 10fps MP4. Adds the multiframe.dcm fixture (8-frame MONOCHROME2 with a VOI window) and ConvertToVideoTest (timing math, format, ffprobe-verified frame counts, fail-loud on missing ffmpeg, temp cleanup, cwd stability). Bakes ffmpeg into the devenv provisioner so CI has it. Suite: 221 passed locally (was 206).
The thin v1 surface over the new Convert::toVideo: writes to
{temp_dir}/{basename(file)}.{format}, supports mp4 only, creates temp_dir if
missing, and returns the output path, softening a marker failure (e.g. a missing
source) to that path the way the other shims do.
One deliberate, safer-than-v1 change, documented in the method: $framerate is
honored as a frames-per-second cine rate. v1 ignored its $framerate argument and
always fed ffmpeg a fixed 10 fps, so callers passing a value got no effect; here
it takes effect. The richer timing/scale/quality/windowing controls live on
Convert::toVideo, not this narrow surface.
Tests cover the output path, mp4-only rejection, temp_dir creation, the
deprecation notice, framerate honoring (ffprobe duration), and the soften-to-path
behavior. Suite: 228 passed.
…rep) The dicom_net shims (echoscu, send_dcm) return the error output string on failure rather than a fixed value, which run()'s fixed onSoftenedFailure could not express. Allow that argument to be a Closure(\Throwable): mixed, called with the caught marker exception to derive the return (typically its message); non-Closure values still return verbatim. Detection is by Closure instance, not is_callable, so a string failure value (an output path, '') is never mistaken for a callback and invoked -- covered by a test passing 'strlen' as the failure value and asserting it returns as-is. The existing convert/tag/function shims pass fixed values and are unchanged. Suite: 231 passed (was 228).
…n() (Phase 6g) The dicom_net compat class over the Phase 5 PACS substrate: - echoscu -> EchoSCU(Peer, Association)->verify(); returns 0 on success, the error string on failure (blackbox-confirmed against v1: 0 / storescu error text). - send_dcm -> SCU::send(File::open(file)) single, or SCU::sendDirectory(dirname) when the 5th arg is truthy. The 5th arg is batch, not debug -- confirmed by blackbox (1 file vs 3 received), correcting an earlier note. Decision (per owner): the substrate stays fail-loud, but the shim matches v1. v1 was procedural and never threw -- where v2 now validates strictly and would abort (e.g. an AE title over 16 chars), run() honors v1's non-fatal behavior by returning the same v1-shaped value and surfacing the rejection as an E_USER_DEPRECATED (not a warning), since it marks usage invalid in v2 going forward. Implemented generally in ShimContract::run via an InvalidArgumentException catch; the failure-value Closure returns the message, so the net shims return it as the error string. Tests run against a real storescp peer (success/unreachable/missing-file/batch) and cover the misuse softening. Suite: 240 passed.
Blackboxing echoscu/storescu showed v1 hardcodes per-tool timeouts (echoscu -ta/-td/-to = 5s; storescu = 10s; storescp -ta/-td = 20s) and storescp also gets --fork -dhl. Per owner decision, expose these as public properties mirroring DCMTK's individual knobs, defaulting to the v1 values: echo_acse/dimse/connection_timeout = 5 send_acse/dimse/connection_timeout = 10 server_acse/dimse_timeout = 20 fork = true, disable_host_lookup = true, blocking = true echoscu/send_dcm now feed their timeout properties into Association (which already supports acse/dimse/connection timeouts -- no substrate change). Verified by capturing the echoscu argv on the v2 path: defaults emit -ta 5 -td 5 -to 5, and overrides emit them independently. The server_* and fork/dhl/blocking properties are the configuration surface the forthcoming store_server consumes. A new test sets an out-of-range timeout to prove the property reaches Association's validation and softens per the contract. Suite: 241 passed.
SCP substrate gains the knobs store_server needs, all optional and defaulting to v2-idiomatic behavior: - presentationConfigFile: storescp -xf <file> Default (else +xa as before) - debug: -v -d - disableHostLookup: -dhl - acse/dimseTimeoutSeconds: -ta/-td (validated >= 1, like Association) start() fails loud (IOException) if the config file is missing. Existing callers pass toolkit by name, so the inserted params are backward-compatible. store_server() drives storescp through SCP: it creates the storage dir (fail-loud on failure), maps the server_*_timeout/fork/disable_host_lookup properties onto the substrate, runs the handler as a bare command with v1's placeholders appended (<handler> #p #f #c #a -- storage dir, file, called AE, calling AE), and honors a presentation config via passthrough. Blocking (default, matching v1's foreground server) waits until the process exits and returns null; blocking=false returns the SCPProcess handle. A startup failure (e.g. port in use) softens to null with a deprecation, matching v1's fall-through. Background: blackboxing dcmtk 3.6.4/3.6.6/3.6.7 showed the example config's rejection was a case typo (RETIRED_VLMultiFrameImageStorage vs ...Multiframe...), not a version boundary -- configs load fine on current dcmtk when correct, so honoring config files via passthrough is sound; the planned version-branch/parser was unnecessary. Tests: SCP gains config-file/missing-config/timeout-validation cases; dicom_net gains handler-placeholder (#p #f #c #a verified against a live send), softened startup failure, and a session-isolated fork proving blocking blocks. Suite: 247.
PresentationContext26 used RETIRED_VLMultiFrameImageStorage (capital F), but DCMTK's keyword is RETIRED_VLMultiframeImageStorage (lowercase f). storescp keyword resolution is case-sensitive, so the one wrong character made the entire config fail to load on every dcmtk tested (3.6.4/3.6.6/3.6.7). With the fix the config loads cleanly.
The 6h.1 completeness audit (reflectV1Surface vs the shim) found dicom_net had drifted from v1's reflected surface; the other three shims matched. Fixes: - Parameter names restored to v1: remote_ae (was target_ae), send_batch (was batch), dcm_dir (was storage_dir). Positional callers were unaffected, but named-argument callers would have broken. - echoscu/send_dcm my_ae/remote_ae now default to 'DEANO' (v1's defaults) instead of being required, so e.g. echoscu($host, $port) works as it did in v1. - transfer_syntax property added. Blackboxing v1 showed it is inert -- setting it to a transfer-syntax UID produced byte-identical storescu argv -- so the shim declares it (also avoiding PHP 8.2+ dynamic-property deprecation) and keeps it inert, emitting an honest deprecation when set non-empty that points to PACS\TransferSyntaxProposal + PACS\SCU, where v2 actually implements transfer- syntax negotiation. config_file stays optional (a benign superset of v1). Test added for the inert-but-deprecated transfer_syntax path. Suite: 248.
Each example now leads with the v1 form (as a 'Before' block) and gives the runnable v2-native 'After' that bypasses the shim -- no deprecations. All require the Composer autoloader instead of the legacy class_dicom.php, so they keep working once that file is deleted. The v2 rewrites lean on the natural improvements the substrate offers: - Typed accessors (getPersonName/getText/getDate/getUID, setText/setPersonName/...) keyed by the Tag enum, instead of v1's raw 'gggg,eeee' addresses, which only existed because v1 had no typed access. get_tags/write_tags/store_server_handler now read and write through them; raw Dataset get/put is shown only as the fallback. - File::transferSyntaxUID() in compress/uncompress instead of reading 0002,0010. - jpg_to_dcm drops the dcm2xml template entirely: Convert::fromJpeg() builds the Secondary Capture object and generates the study/series/SOP UIDs, then typed setters fill identifying tags. The old jpg_to_dcm.xml is now obsolete. - send_dcm notes the real transfer-syntax control v1 only pretended to have (PACS\TransferSyntaxProposal), since the shim's transfer_syntax is inert. - store_server uses PACS\SCP; the handler corrects v1's mislabeled AE arguments (#c is the called/receiver AE, #a the calling/sender AE). All examples were smoke-tested: the read/convert/compress ones run against the bundled fixtures (writing to temp so fixtures are never mutated), and the net ones against live storescp peers/servers on local ports (send, directory send-and-move, and a store server whose handler logged 'called AE example <- calling AE DEANO').
Replaces the v1-oriented README with the v2 namespaced API as the primary surface, keeping Dean Vaughan's provenance and the DCMTK/OFFIS acknowledgment. - Intro frames v2 as the PHP 8.5 clean rewrite with the v1 procedural surface kept as a deprecating compatibility shim. - Requirements: drop the TOOLKIT_DIR-constant note for DCMTK\Toolkit (PATH by default, explicit directory via the constructor); add ffmpeg for video. - Installation is Composer-only; removed the copy-and-require-the-file instructions. - Quick start is entirely v2-native: typed accessors, Convert/Compress, transferSyntaxUID(), toVideo(), and EchoSCU/SCU/SCP with TransferSyntaxProposal. - New 'Migrating from v1' section: the shim deprecates, points at the examples and docs/migration-v1-to-v2.md, and flags the raw-address, XML-template, and inert transfer_syntax gotchas. - Removed the obsolete jpg_to_dcm 'Known issue' block (v2 fixes it). - Testing points at composer test (PHPUnit + pydicom/pynetdicom oracles) instead of the old python runner. API reference is the v2 namespace table plus a shim->v2 map. The docs/migration-v1-to-v2.md link is intentionally ahead of its file (Phase 6h.5); both land before the Phase 6 PR.
Compress::compress() defaults to lossless SV1 (JPEG process 14, SV1), which is v1's actual default. The README and examples/compress.php had used Compression::lossless() (plain JPEG lossless, a different transfer syntax: ...4.57 vs ...4.70), which contradicted the 'matching v1' note. Both now call losslessSV1() and produce 1.2.840.10008.1.2.4.70 as v1 did.
docs/migration-v1-to-v2.md maps every v1 surface element to its v2 home, built from the shim's own deprecation targets. Covers the global functions, dicom_tag, dicom_convert, and dicom_net, with each row linking to its worked example in examples/. Notes the behavior changes that matter when going native: the error model flips from sentinel returns to thrown exceptions, transfer_syntax was inert (use TransferSyntaxProposal), the jpg_to_dcm XML template and temp_dir are gone, compress defaults to lossless SV1, and the handler placeholder order is #p #f #c #a. Closes with the Toolkit replacement for TOOLKIT_DIR and how to confirm a migration is complete (run with deprecations visible). This is the doc the README links and the seed for Phase 7.
The v2 rewrite is complete and real-world consumers are migrated, so the v1-era scaffolding is no longer needed: - class_dicom.php: the legacy v1 library (nothing depended on it; the compat shims are independent v2 implementations, not wrappers of it) - tools/research/reflectV1Surface.php, exerciseV1Surface.php: the clean-room reflection/exercise tooling that froze the v1 surface - tests/test_class_dicom.py, tests/dicom_scp.py: the v1 blackbox oracle test and its SCP helper (never part of the phpunit CI suite) - examples/jpg_to_dcm.xml: obsolete dcm2xml template (the v2 example generates UIDs directly) - docs/v1-surface.json: the frozen reflection dump (recoverable from history) Docs updated to match: tools/README.md trimmed to the two remaining DCMTK blackbox helpers, and the two markdown links to v1-surface.json de-linked to code spans. The clean-room provenance comments in the shims and the historical rewrite/capability narratives are kept intentionally.
rbraunm
approved these changes
Jun 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes the v2 rewrite by adding the v1 compatibility surface, then removing the v1 scaffolding that is no longer needed.
The bulk is the compat layer (Phase 6a-6g): non-namespaced shims (is_dcm, Execute, dicom_tag, dicom_convert, dicom_net) that reproduce v1's public API verbatim on top of v2, so existing v1-style callers keep working unchanged. dicom_net was aligned to the frozen v1 contract -- v1 parameter names, the DEANO defaults, and an inert transfer_syntax property that points callers at PACS\TransferSyntaxProposal. Video assembly (multiframe_to_video) is backed by a real Convert::toVideo in the v2 core rather than faked in the shim.
Two decisions worth noting. The shims are independent v2 implementations, not wrappers of the legacy file -- clean-room throughout, the original source was never read, only loaded and run as a blackbox. And the storescp config is passed through after confirming empirically that the earlier example rejection was a case typo, not a DCMTK version boundary.
Phase 6h rewrote the README and examples v2-first (the examples are now before/after v1->v2 migration recipes) and added a per-class migration map, then the teardown removed the legacy class_dicom.php, the clean-room reflection tooling, the v1 oracle test, and the obsolete XML template -- all recoverable from history. The clean-room provenance comments and historical rewrite narratives are kept.
Where it stands: CI green (PHP 8.5 + DCMTK). Real-world consumers were migrated separately, which confirmed this surface is complete.