Skip to content

fix(video): Dolby Vision crash, 50% faster transcode, fps/bitrate/GPS fixes#399

Open
HuuNguyen312 wants to merge 5 commits into
numandev1:mainfrom
HuuNguyen312:main
Open

fix(video): Dolby Vision crash, 50% faster transcode, fps/bitrate/GPS fixes#399
HuuNguyen312 wants to merge 5 commits into
numandev1:mainfrom
HuuNguyen312:main

Conversation

@HuuNguyen312
Copy link
Copy Markdown

@HuuNguyen312 HuuNguyen312 commented May 14, 2026

Summary

Bug (#398) — Dolby Vision crash on .MOV from iPhone:
createDecoderByType("video/dolby-vision") throws on Android — no standalone DV decoder. DV profile 8.x has HEVC base layer, so remap mime → video/hevc. Reject profile 5 (0x20) explicitly (no HEVC fallback).

Perf (#384) — ~50% faster transcode:

  • HW AVC encoder via MediaCodecList(ALL_CODECS), blacklist c2.qti.avc.encoder (corrupt MP4).
  • Feed decoder until input slots drain (parallel decode-render-encode).
  • Drop frames only when source fps > target fps; anchor next slot to ideal grid, not actual PTS.
  • Encoder: VBR + KEY_PRIORITY=0 + KEY_OPERATING_RATE=MAX.
  • SurfaceTexture.onFrameAvailable on dedicated HandlerThread.
  • Skip StreamableVideo rewrite when no streamableFile passed.

fps detection:

  • Derive source fps from frame_count / duration when CAPTURE_FRAMERATE absent (most non-slo-mo clips returned 0, forced 30fps cap).
  • Raise fps cap 30 → 60 on Android + iOS.

Bitrate:

  • WhatsApp-style envelope (~1.5 Mbps @ 720p). Previous bands produced 20-40 MB for short clips. Android + iOS in sync.

GPS preservation:

  • LocationExtractor walks raw MP4 box tree: handles ©xyz, ISO loci, iTunes meta/keys+ilst, Samsung SEF trailer regex.
  • MediaMetadataRetriever misses vendor-specific placements; extractor covers the gaps.
  • iOS: forward asset.metadata + all availableMetadataFormats to AVAssetExportSession.

Stability:

  • dispose() wraps every step in runCatching — teardown failure no longer leaks codec handles.
  • OutputSurface.release() joins HandlerThread after quitSafely() — prevents SIGABRT on stale pthread_t.

Changelog

[ANDROID] [FIXED] Dolby Vision .MOV no longer crashes (#398)
[ANDROID] [CHANGED] Transcode pipeline ~50% faster (#384)
[ANDROID] [FIXED] Source fps derived from frame count when CAPTURE_FRAMERATE absent
[ANDROID] [FIXED] GPS metadata preserved via raw MP4 box walker + Samsung SEF scan
[iOS] [FIXED] fps cap raised to 60; bitrate bands halved; source metadata forwarded
[ANDROID] [FIXED] Codec teardown wrapped in runCatching; OutputSurface thread joined


Test Plan

  • yarn test:pr
  • yarn test:harness:android
  • iPhone HDR .MOV (DV profile 8.4) — was crash, now transcodes
  • 500MB MP4: ~60s → ~30s on Android
  • Output MP4 plays on macOS QuickTime + iOS Photos
  • 60fps source output stays 60fps (not halved to 30fps)
  • Output file retains GPS coordinates from source
  • yarn test:harness:ios

HuuNguyen312 and others added 3 commits May 14, 2026 10:04
  iPhone HDR .MOV uses video/dolby-vision mime which has no
  standalone Android decoder, so createDecoderByType throws
  "Failed to initialize video/dolby-vision". DV profiles 8.x
  carry an HEVC base layer, so remap mime to video/hevc before
  configuring the decoder. Reject profile 5 (0x20) explicitly
  since it has no HEVC fallback.

  Perf, bundled to land with the codec rework:
  - Pick HW AVC encoder via MediaCodecList(ALL_CODECS), blacklist
    c2.qti.avc.encoder (corrupt MP4 on Mac/iOS).
  - Feed decoder until input slots drain instead of one sample
    per loop; unblocks parallel decode-render-encode.
  - Drop decoded frames whose PTS precedes the next target slot
    when source fps exceeds output fps.
  - Encoder: VBR + KEY_PRIORITY=0 + KEY_OPERATING_RATE=MAX to
    unthrottle HW codec scheduling.
  - Route SurfaceTexture onFrameAvailable to a dedicated
    HandlerThread so awaitNewImage stops contending with the
    main/JS thread.
  - Skip StreamableVideo rewrite unless caller passed a
    streamableFile; halves disk I/O for chat uploads.
Android: extract METADATA_KEY_LOCATION and write an Apple-style "©xyz"
udta atom into the muxed MP4 so geotags survive transcoding.
iOS: forward asset.metadata plus every available metadata format to the
AVAssetExportSession so location, creation date, and other tags are
retained in the exported file.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- fps: derive from frame_count/duration when CAPTURE_FRAMERATE absent.
  Cap 30→60. Drop-gate only when source>target, anchor to ideal grid.
- bitrate: WhatsApp envelope (~1.5 Mbps @ 720p). Android+iOS sync.
- GPS: LocationExtractor walks MP4 — ©xyz, loci, iTunes meta/keys+ilst,
  SEF trailer regex. Writer ©xyz moved to LocationBox class.
- teardown: runCatching every dispose step. join() OutputSurface thread
  after quitSafely to avoid SIGABRT on stale pthread_t.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@HuuNguyen312 HuuNguyen312 changed the title fix(android): support Dolby Vision MOV and speed up transcode fix(video): Dolby Vision crash, 50% faster transcode, fps/bitrate/GPS fixes May 15, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the video (and some image) compression pipelines to improve Android stability/performance and preserve important metadata across transcodes, while aligning bitrate/fps behavior across Android and iOS.

Changes:

  • Fix Android Dolby Vision .MOV crashes by remapping video/dolby-vision to HEVC decoding where possible, and tightening codec selection/teardown behavior.
  • Improve Android transcode throughput via better decoder feeding, frame dropping logic when downsampling fps, dedicated SurfaceTexture callback thread, and hardware AVC encoder selection.
  • Align output characteristics and metadata handling: raise fps cap to 60, reduce bitrate “envelope”, and preserve GPS/location/asset metadata (Android MP4 ©xyz + iOS exporter metadata forwarding). Also improves Android image EXIF copy path handling.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ios/Video/VideoMain.swift Raises fps cap to 60, halves bitrate bands, and forwards asset metadata into the exporter.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressorHelper.kt Adds derived fps detection fallback (frameCount/duration) and uses it in manual compression.
android/src/main/java/com/reactnativecompressor/Video/AutoVideoCompression.kt Switches auto compression to use the new derived fps detection helper.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressor/videoHelpers/OutputSurface.kt Moves onFrameAvailable onto a dedicated HandlerThread, improves teardown, and increases frame wait timeout.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressor/videoHelpers/Mp4Movie.kt Adds a location field to carry GPS through MP4 building.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressor/videoHelpers/MP4Builder.kt Writes GPS into moov/udta/©xyz when available to preserve location metadata.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressor/videoHelpers/LocationBox.kt Introduces an MP4 ©xyz box implementation for persisting ISO 6709 location strings.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressor/utils/LocationExtractor.kt Adds a raw MP4 box walker + Samsung trailer scan to extract GPS when retriever misses it.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressor/utils/CompressorUtils.kt Adds MP4 movie setup support for location and switches encoder bitrate mode/settings for throughput.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressor/compressor/Compressor.kt Implements DV mime remap, improved decoder feeding, conditional frame dropping, smarter encoder selection, streamable rewrite gating, and safer teardown.
android/src/main/java/com/reactnativecompressor/Video/VideoCompressionProfile.kt Raises max fps cap to 60 and updates bitrate bands to match iOS envelope.
android/src/main/java/com/reactnativecompressor/Image/ImageCompressor.kt Normalizes URI strings to filesystem paths for EXIF operations and ensures output stream is closed before EXIF writes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +46 to +49
Log.i(
TAG,
"LocationExtractor box scan: xyz=${state.xyz} itunes=${state.itunesLocation} loci=${state.loci} chosen=$viaBox"
)
Comment on lines +111 to +118
val retrievedLocation =
mediaMetadataRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_LOCATION)
val locationData = if (!retrievedLocation.isNullOrEmpty()) {
retrievedLocation
} else {
LocationExtractor.extract(context, srcUri)
}
Log.i("Compressor", "source location resolved: $locationData (retriever=$retrievedLocation)")
Comment on lines +690 to +696
-1
}
// DV profile 5 = 0x20, no HEVC fallback. Profiles 8.x carry HEVC base layer.
if (profile == 0x20) {
throw IllegalStateException("Dolby Vision profile 5 has no HEVC base layer; cannot transcode")
}
inputFormat.setString(MediaFormat.KEY_MIME, MediaFormat.MIMETYPE_VIDEO_HEVC)
} else {
LocationExtractor.extract(context, srcUri)
}
Log.i("Compressor", "source location resolved: $locationData (retriever=$retrievedLocation)")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserving the source location in the compressed video is fine, but we should not print the exact gps/location string in logcat because it exposes user coordinates

please log only something like hasLocation=true or locationSource=retriever/extractor

val viaBox = chooseBest(state)
Log.i(
TAG,
"LocationExtractor box scan: xyz=${state.xyz} itunes=${state.itunesLocation} loci=${state.loci} chosen=$viaBox"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logs the actual extracted gps values like xyz, itunes, loci and chosen location

location preservation is okay, but exact coordinates should not be logged

please replace this with booleans like hasXyz, hasItunesLocation, hasLoci and hasChosenLocation

buf.get(bytes)
val text = String(bytes, StandardCharsets.ISO_8859_1)
val match = ISO6709_REGEX.find(text)
Log.i(TAG, "LocationExtractor SEF trailer scan match=${match?.value}")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can print the exact gps value found from the samsung sef trailer

please avoid logging match.value and log only whether a match was found

}
// DV profile 5 = 0x20, no HEVC fallback. Profiles 8.x carry HEVC base layer.
if (profile == 0x20) {
throw IllegalStateException("Dolby Vision profile 5 has no HEVC base layer; cannot transcode")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this throw can happen after the encoder and surfaces are already created and started

because the catch returns without calling dispose, encoder and egl resources can leak and may break the next compression

please preflight dolby vision before creating encoder/inputSurface/outputSurface or add a finally cleanup path for partially initialized resources

// StreamableVideo rewrites the whole MP4 to move the moov atom to the front,
// which doubles disk I/O. Only run it when the caller explicitly requested a
// streamable copy (non-null streamableFile). Chat uploads do not need it.
if (streamableFile != null) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changes the previous default behavior

before this PR, StreamableVideo was also run when streamableFile was null, so default compressed mp4 output was browser/progressive playback friendly by moving the moov atom to the front

please confirm this behavior change is intentional or keep the old default behavior

setInteger(
MediaFormat.KEY_BITRATE_MODE,
MediaCodecInfo.EncoderCapabilities.BITRATE_MODE_CBR
MediaCodecInfo.EncoderCapabilities.BITRATE_MODE_VBR
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this forces vbr, priority and max operating rate for every encoder

some android encoders may reject these settings during configure

please guard this with encoder capability checks or fallback to cbr/default settings if configure fails

…encoder/teardown

- LocationExtractor/Compressor: log only location presence + source, never the
  ISO 6709 coordinate string (xyz/itunes/loci/SEF/resolved values)
- Compressor: preflight Dolby Vision profile 5 before allocating
  muxer/encoder/EGL surfaces and drop the throw from prepareDecoder, so the
  unsupported case no longer leaks codec/GL resources on bail-out
- Compressor: restore always-on streamable rewrite (moov atom to front) for
  default output to preserve progressive playback (revert behavior change)
- CompressorUtils/Compressor: make VBR/priority/operating-rate throughput
  tuning optional and fall back to a default-rate-control configure when an
  encoder rejects the tuned format
- Compressor: release partially-initialized encoder/decoder/EGL surfaces on any
  setup failure or in-loop throw (dispose tolerates null handles)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@HuuNguyen312
Copy link
Copy Markdown
Author

@numandev1 I've resolved all the comments, please help me review them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants