refactor(hls): trust the upload server URL verbatim, drop withExtensionHint
Policy shift agreed with the server-side collaborator: the URL the
NIP-96 / Blossom server returns is already the optimal form — clean
Content-Type, correct cache keys, range-friendly — and the client
should not second-guess it. Stripping extensions, appending hints or
rewriting to bare sha256 all add round trips and cache misses for no
real win now that the WordPress plugin returns clean <hash>.m3u8 URLs.
Concretely:
- HlsUploadPipeline drops the withExtensionHint helper entirely. Every
combinedUrl / mediaPlaylistUrl / masterUrl flows straight from the
uploader's MediaUploadResult.url into the playlist rewrite and the
HlsUploadResult.
- A small kdoc note at the top of the pipeline documents the policy
("if a server returns an unplayable URL, the fix is server-side").
Pipeline tests pruned:
- appendsMp4HintToBareCombinedUrlInMediaPlaylist — removed, was testing
the appender that no longer exists.
- appendsM3u8HintToBarePlaylistUrlInMasterPlaylist — removed, same.
- trailingDotUrlsAreSanitisedIntoCleanExtension — removed, the Nip96
fallback-extension map keeps the server from returning trailing-dot
URLs in the first place.
- doesNotDoubleAppendExtensionWhenAlreadyPresent — removed.
Kept and reframed:
- serverUrlsAlreadyWithExtensionPassThroughUntouched — the canonical
contract: server returns clean .m3u8 / .mp4, pipeline forwards
verbatim.
- bareServerUrlsPassThroughVerbatim (new) — explicitly documents that
even a bare-hash URL flows through unchanged; the pipeline does not
try to make it playable.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
+9
-34
@@ -58,6 +58,12 @@ data class HlsUploadedRendition(
|
||||
* Finally rewrites the master playlist to reference the per-rendition playlist URLs and uploads
|
||||
* the master. The resulting [HlsUploadResult] is what the publisher uses to build the NIP-71
|
||||
* event.
|
||||
*
|
||||
* URL handling policy: the pipeline uses the URL the server returned verbatim. No extension is
|
||||
* appended, no trailing dot stripped, no bare-hash rewriting. The server is responsible for
|
||||
* returning a URL that the player can fetch as-is — that way we get the best cache coherence,
|
||||
* correct Content-Type, clean range requests, and no double round trips. If a server returns an
|
||||
* unplayable URL, the fix is server-side.
|
||||
*/
|
||||
class HlsUploadPipeline(
|
||||
private val uploader: HlsBlobUploader,
|
||||
@@ -75,10 +81,7 @@ class HlsUploadPipeline(
|
||||
val combined = uploader.upload(rendition.combinedFile, CONTENT_TYPE_VIDEO_MP4)
|
||||
onProgress(++done, total)
|
||||
val combinedUrl =
|
||||
withExtensionHint(
|
||||
combined.url ?: error("Uploader returned null URL for ${rendition.combinedFile.name}"),
|
||||
CONTENT_TYPE_VIDEO_MP4,
|
||||
)
|
||||
combined.url ?: error("Uploader returned null URL for ${rendition.combinedFile.name}")
|
||||
|
||||
val rewrittenMedia =
|
||||
HlsPlaylistRewriter.rewrite(
|
||||
@@ -90,10 +93,7 @@ class HlsUploadPipeline(
|
||||
val mediaPlaylist = uploader.upload(mediaPlaylistFile, CONTENT_TYPE_HLS)
|
||||
onProgress(++done, total)
|
||||
val mediaPlaylistUrl =
|
||||
withExtensionHint(
|
||||
mediaPlaylist.url ?: error("Uploader returned null URL for media playlist ${rendition.label}"),
|
||||
CONTENT_TYPE_HLS,
|
||||
)
|
||||
mediaPlaylist.url ?: error("Uploader returned null URL for media playlist ${rendition.label}")
|
||||
|
||||
HlsUploadedRendition(
|
||||
label = rendition.label,
|
||||
@@ -112,10 +112,7 @@ class HlsUploadPipeline(
|
||||
val master = uploader.upload(masterFile, CONTENT_TYPE_HLS)
|
||||
onProgress(++done, total)
|
||||
val masterUrl =
|
||||
withExtensionHint(
|
||||
master.url ?: error("Uploader returned null URL for master playlist"),
|
||||
CONTENT_TYPE_HLS,
|
||||
)
|
||||
master.url ?: error("Uploader returned null URL for master playlist")
|
||||
|
||||
return HlsUploadResult(
|
||||
masterUrl = masterUrl,
|
||||
@@ -124,28 +121,6 @@ class HlsUploadPipeline(
|
||||
)
|
||||
}
|
||||
|
||||
// Blossom servers typically return bare-hash URLs (https://server/<sha256>), but HLS parsers
|
||||
// and ExoPlayer's Util.inferContentType sniff the URL extension to pick the right source
|
||||
// factory. Append a hint unless the upload server already baked one in. Trailing dots are
|
||||
// trimmed and any pre-existing doubled extension (e.g. "..m3u8") collapsed, so a server that
|
||||
// echoes our empty-extension upload filename does not produce unreachable URLs.
|
||||
private fun withExtensionHint(
|
||||
url: String,
|
||||
contentType: String,
|
||||
): String {
|
||||
val ext =
|
||||
when (contentType) {
|
||||
CONTENT_TYPE_VIDEO_MP4 -> ".mp4"
|
||||
CONTENT_TYPE_HLS -> ".m3u8"
|
||||
else -> return url
|
||||
}
|
||||
val sanitised =
|
||||
url
|
||||
.replace(Regex("""\.{2,}""" + Regex.escape(ext.trimStart('.')) + "$", RegexOption.IGNORE_CASE), ext)
|
||||
.trimEnd('.')
|
||||
return if (sanitised.endsWith(ext, ignoreCase = true)) sanitised else sanitised + ext
|
||||
}
|
||||
|
||||
companion object {
|
||||
const val CONTENT_TYPE_VIDEO_MP4 = "video/mp4"
|
||||
const val CONTENT_TYPE_HLS = "application/vnd.apple.mpegurl"
|
||||
|
||||
+11
-62
@@ -180,36 +180,25 @@ class HlsUploadPipelineTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun appendsMp4HintToBareCombinedUrlInMediaPlaylist() {
|
||||
val bundle = createBundle(listOf("360p"))
|
||||
val uploader = BareUrlUploader()
|
||||
val pipeline = HlsUploadPipeline(uploader)
|
||||
|
||||
runBlocking { pipeline.upload(bundle) }
|
||||
|
||||
val uploadedMediaPlaylist = uploader.calls[1].third
|
||||
assertTrue(
|
||||
"media playlist should reference url with .mp4 hint",
|
||||
uploadedMediaPlaylist.contains("https://blossom.test/bare-1.mp4"),
|
||||
)
|
||||
assertTrue(!uploadedMediaPlaylist.contains("https://blossom.test/bare-1\""))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun appendsM3u8HintToBarePlaylistUrlInMasterPlaylist() {
|
||||
fun bareServerUrlsPassThroughVerbatim() {
|
||||
// Policy: the pipeline uses whatever URL the server returned, unchanged.
|
||||
// Even a bare-hash URL with no extension flows straight into the rewritten
|
||||
// playlists. If it does not play, the fix is server-side (return a playable URL).
|
||||
val bundle = createBundle(listOf("360p"))
|
||||
val uploader = BareUrlUploader()
|
||||
val pipeline = HlsUploadPipeline(uploader)
|
||||
|
||||
val result = runBlocking { pipeline.upload(bundle) }
|
||||
|
||||
val uploadedMaster = uploader.calls[2].third
|
||||
assertEquals("https://blossom.test/bare-1", result.renditions[0].combinedUrl)
|
||||
assertEquals("https://blossom.test/bare-2", result.renditions[0].playlistUrl)
|
||||
assertEquals("https://blossom.test/bare-3", result.masterUrl)
|
||||
// The rewritten media playlist that the server received must contain the bare url.
|
||||
val uploadedMediaPlaylist = uploader.calls[1].third
|
||||
assertTrue(
|
||||
"master playlist should reference playlist url with .m3u8 hint",
|
||||
uploadedMaster.contains("https://blossom.test/bare-2.m3u8"),
|
||||
"media playlist should reference bare url: $uploadedMediaPlaylist",
|
||||
uploadedMediaPlaylist.contains("https://blossom.test/bare-1"),
|
||||
)
|
||||
assertEquals("https://blossom.test/bare-3.m3u8", result.masterUrl)
|
||||
assertEquals("https://blossom.test/bare-1.mp4", result.renditions[0].combinedUrl)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -252,46 +241,6 @@ class HlsUploadPipelineTest {
|
||||
assertTrue(!result.renditions[0].playlistUrl.contains(".m3u8.m3u8"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun trailingDotUrlsAreSanitisedIntoCleanExtension() {
|
||||
val bundle = createBundle(listOf("360p"))
|
||||
val uploader =
|
||||
object : HlsBlobUploader {
|
||||
var count = 0
|
||||
|
||||
override suspend fun upload(
|
||||
file: File,
|
||||
contentType: String,
|
||||
): MediaUploadResult {
|
||||
count++
|
||||
return MediaUploadResult(url = "https://blossom.test/bare-$count.", sha256 = "sha-$count", size = file.length())
|
||||
}
|
||||
}
|
||||
val pipeline = HlsUploadPipeline(uploader)
|
||||
|
||||
val result = runBlocking { pipeline.upload(bundle) }
|
||||
|
||||
assertTrue("masterUrl must not contain '..': ${result.masterUrl}", !result.masterUrl.contains(".."))
|
||||
assertTrue("masterUrl must not end with '.': ${result.masterUrl}", !result.masterUrl.endsWith("."))
|
||||
assertTrue(result.masterUrl.endsWith(".m3u8"))
|
||||
assertTrue(!result.renditions[0].combinedUrl.contains(".."))
|
||||
assertTrue(result.renditions[0].combinedUrl.endsWith(".mp4"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun doesNotDoubleAppendExtensionWhenAlreadyPresent() {
|
||||
val bundle = createBundle(listOf("360p"))
|
||||
val uploader = FakeUploader() // returns urls ending in .mp4 / .m3u8
|
||||
val pipeline = HlsUploadPipeline(uploader)
|
||||
|
||||
runBlocking { pipeline.upload(bundle) }
|
||||
|
||||
val uploadedMediaPlaylist = uploader.calls[1].content
|
||||
assertTrue(!uploadedMediaPlaylist.contains(".mp4.mp4"))
|
||||
val uploadedMaster = uploader.calls[2].content
|
||||
assertTrue(!uploadedMaster.contains(".m3u8.m3u8"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun reportsUploadProgressPerStep() {
|
||||
val bundle = createBundle(listOf("360p", "540p"))
|
||||
|
||||
Reference in New Issue
Block a user