Skip to content

Persisting a language as ISO 639-2/B drops the region and script the tag carries #136

Description

@malard

A stream detail language is a CLanguageTag in memory, but every place it is persisted narrows
it to ISO 639-2/B on the way out. That form holds less than the tag does, and the loss is
permanent: the value read back is not the value that was scanned.

Where it happens

Three surfaces, all writing the same projection:

// VideoDatabase.cpp, SetStreamDetailsForFileId()
details.GetVideoLanguage(i).AsIso6392B().c_str(),      // strVideoLanguage
details.GetAudioLanguage(i).AsIso6392B().c_str(),      // strAudioLanguage
details.GetSubtitleLanguage(i).AsIso6392B().c_str(),   // strSubtitleLanguage

// StreamDetails.cpp, CStreamDetail{Video,Audio,Subtitle}::Archive()
ar << m_language.AsIso6392B();

// VideoInfoTag.cpp, Save() - NFO export
XMLUtils::SetString(&stream, "language", m_streamDetails.GetAudioLanguage(iStream).AsIso6392B());

The read path parses whatever came back:

// VideoDatabase.cpp, GetStreamDetails()
p->m_language = KODI::UTILS::CLanguageTag::Parse(pDS->fv(8).get_asString());

What the round trip loses

ISO 639-2/B names a language and nothing else, so every subtag qualifying it is dropped:

scanned persisted read back as
pt-BR por pt
pt-PT por pt
zh-Hant chi zh
zh-Hans chi zh
en-AU eng en

The first two pairs and the second two pairs are not distinguishable after a write. Traditional
and Simplified Chinese subtitle tracks become one language; Brazilian and European Portuguese
audio become one language.

Why this matters more than it did

The in-memory type now carries the whole tag, and TestStreamDetails.SerializeWidensLanguageToBcp47
asserts JSON-RPC serves it. That promise holds only until the item is reloaded from the library:
scan gives pt-BR, the database gives back pt, and the same API answers differently depending
on whether the item happens to be in memory from the scan. The narrowing is now the only place
the region is lost.

What a fix needs

  • streamdetails: store ToString(). A schema migration, and the columns already hold
    por-style values that CLanguageTag::Parse reads correctly, so no data conversion is
    strictly required - old rows simply keep the loss they already have.
  • CArchive: store ToString(). The archive is a cache, so a version bump that invalidates
    it is enough; there is no migration to write.
  • NFO: read is already tolerant - ParseStreamLanguage accepts either form. Writing the tag
    is an ecosystem question rather than a technical one, since other tools read these files, and
    should be decided separately from the two above.

Note on the epic

This epic's body currently states the opposite as a rule:

Storage and wire formats are contracts and do not move with the type. The streamdetails
language columns stay ISO 639-2/B; JSON-RPC gains BCP 47 additively rather than by changing an
existing field's notation.

That rule has since been overtaken. Application.GetProperties now returns the tag rather than a
POSIX locale, and the language infolabels return the tag rather than an ISO 639-2 code, both as
deliberate breaks carried by the JSON-RPC v14 and skinning v22 version gates. The epic text needs
updating either way; this ticket is the storage half of the same question.

Found while auditing every As*() call site for whether its result is consumed at the boundary
that asked for it. Thirty-six of thirty-seven were; these three write a projection to disk, which
is the one place it cannot be undone.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: i18nLanguage codes, BCP 47, localisationstatus: investigatedRoot cause established and cited; implementation not startedtype: gapMissing capability rather than a defect

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions