Skip to content

SdpTransform Refactor Log

Melih Ercan edited this page Sep 7, 2026 · 1 revision

Utilme.SdpTransform — Refactor Log

What changed on the way to 26.9.7, and what is left.

Fixed

Every defect below was first pinned by a characterization test asserting the broken behaviour, then fixed, with that test rewritten rather than deleted so the change is visible in the diff. KnownDefectTests, which held them, is now empty and gone.

Fixed in Phase 2 (writer)

All five writer defects are fixed. Their tests were rewritten and moved to WriterRegressionTests, and the round trip is now asserted byte for byte rather than line-by-line-trimmed.

Was Now
b=AS:1024 written back as b=ApplicationSpecific 1024 writes the Display name and :, so it parses back
a=extmap:1/sendonly written as a=extmap:1sendonly the / separator is preserved
NullReferenceException writing an attribute-free SDP optional Attributes and MediaDescriptions are handled
12 lines with stray trailing whitespace none
a=rid:hi send ;max-width=1280 pt= and restrictions form one ;-separated list

Fixed in Phase 3 (parser)

Was Now
enum matching case-sensitive, so RFC-spelled UDP threw matching ignores case; output is still written in canonical casing
whitespace-only lines reported as unsupported fields trimmed before blanks are discarded, so they are ignored silently
string.Replace stripped indicators anywhere in a value stripped from the start only
k=prompt threw IndexOutOfRangeException accepted; Value is null and it round-trips
ToSeconds threw for m and s suffixes all four of d, h, m, s scale correctly

The anchoring fix is the one change in this phase that alters the result for input that already parsed: "s=a s=b".ToSessionName() returned "a b" and now returns "a s=b". The old behaviour silently corrupted any value containing an indicator-like substring — a session name, a URI query with u= in it, an fmtp parameter.

Fixed in Phase 4 (diagnostics)

Console.WriteLine is gone and the swallowed exception is gone. ToSdpResult reports both:

var result = sdpText.ToSdpResult();

if (!result.IsValid)
    foreach (var d in result.Diagnostics)
        Console.Error.WriteLine(d);        // "Error: IndexOutOfRangeException: ..."
else
    foreach (var d in result.Diagnostics)  // warnings for lines that were skipped
        _log.Warn(d.ToString());           // "Warning (line 6): Unsupported session field: x=bad"
Was Now
unknown fields written to Console.WriteLine reported as Warning diagnostics the caller can act on
exception caught, message assigned to an unused local, null returned reported as an Error diagnostic naming the exception type and message
session-level warnings mislabelled "media description attribute" scope named correctly
no way to know which line was at fault 1-based line numbers into the original text, counting blanks

ToSdp is unchanged — same signature, still null on failure — and is now a one-line wrapper over ToSdpResult. Parsing remains all-or-nothing: a recognised indicator whose value does not parse still aborts the whole document, so nothing that parsed before parses differently now.

Fixed in Phase 5 (structural)

Session and media attributes are handled symmetrically. Both scopes now share one parser dispatcher and one writer, so they cannot drift apart again. Attributes valid at session level — a=setup, a=sendonly, a=mid, a=rtpmap and the rest — are parsed and written instead of being dropped with a warning; a=ice-lite, previously session-only, is accepted at media level too.

The writer had to change with the parser: capturing more at session level without emitting it would have silently dropped those lines on the way back out.

ModelExtensions.cs was split from 1289 lines into five partial-class files — .Parsing, .Writing, .Fields, .Attributes, .Utility. No API change; partial is a compile-time construct.

Outstanding

Enum names are still duplicated. Every member carries both a [JsonStringEnumMemberName] and a [Display] name, maintained independently and expected to be identical.

The drift risk is gone — EnumNameTests now asserts that both attributes exist on every member of every SDP enum, that they agree, that none is blank, that no two are ambiguous under the case-insensitive matching introduced in Phase 3, and that each round-trips through the library's own lookup. The duplication itself remains, because collapsing it means deleting [Display] from public enum members, and that is public metadata a consumer could in principle reflect on.

Clone this wiki locally