{CUDA Decoding} Add NvCodecConfig.h, migrate WITH_NVCODEC guards to XPRS_HAS_NVDEC/XPRS_HAS_NVENC (#259) - #259
Closed
YLouWashU wants to merge 2 commits into
Closed
Conversation
Contributor
|
@YLouWashU has exported this pull request. If you are a Meta employee, you can view the originating Diff in D103253723. |
YLouWashU
force-pushed
the
export-D103253723
branch
from
July 28, 2026 01:07
b6bb3c6 to
f64fc3d
Compare
YLouWashU
added a commit
to YLouWashU/vrs-1
that referenced
this pull request
Jul 30, 2026
…PRS_HAS_NVDEC/XPRS_HAS_NVENC (facebookresearch#259) Summary: This is D1b in the GPU-accelerated H.265 decoding stack for projectaria-tools (plan v11 at ~/gdrive/plans/2026-04-03-gpu-accelerated-h265-decoding-pat-v11.md, tech design at https://docs.google.com/document/d/1TMeLy0TqvAdlYo0IY3Qm4BrzE8i035z9MzYvGl_CzeM/edit by Lou Yang). Refactor only — no behavior change. Replaces 22 in-source `#ifdef WITH_NVCODEC` directives with self-documenting `XPRS_HAS_NVDEC` (decoder side) and `XPRS_HAS_NVENC` (encoder side) macros, defined by a new `NvCodecConfig.h` single-source-of-truth header. Build-system flags (`-DWITH_NVCODEC=1` in BUCK and CMake) remain as the upstream input. Why split the macro: today both halves are gated together (`WITH_NVCODEC` is set or unset for both encoder and decoder). The split prepares for build configurations that ship only one half — most importantly the OSS PyPI wheel will want NVDEC without NVENC, since PyPI users typically want decode acceleration but the encode path requires hardware NVIDIA explicitly doesn't support across all SKUs (e.g., A100 has NVDEC but no NVENC). At the call site, `#ifdef XPRS_HAS_NVDEC` makes the intent clear in a way that `#ifdef WITH_NVCODEC` does not. Files changed: NEW arvr/projects/compression/xprs/NvCodecConfig.h — defines XPRS_HAS_NVDEC + XPRS_HAS_NVENC under WITH_NVCODEC, with explanatory comment. arvr/projects/compression/xprs/Codecs.h — removed `#include "nvEncoder.h"` (its only consumers — xprsEncApi.cpp, xprsEncoder.cpp, xprs_gtest_common.h — already include nvEncoder.h directly). Adds an explanatory comment so the next reader doesn't re-add it. arvr/projects/compression/xprs/xprsDecApi.cpp — added `#include "NvCodecConfig.h"`, replaced 4× `WITH_NVCODEC` with `XPRS_HAS_NVDEC`. arvr/projects/compression/xprs/xprsDecoder.h — added include, swapped the nvDecoder.h gate to XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsDecoder.cpp — added include, consolidated two redundant adjacent `#ifdef WITH_NVCODEC` blocks (one wrapping `cudaContextProvider.h`, one wrapping `Codecs.h` — Codecs.h was unconditionally needed and didn't actually need a guard), used XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsEncApi.cpp — added include, replaced 6× `WITH_NVCODEC` with `XPRS_HAS_NVENC`, updated TODO comment to reference the new macro. arvr/projects/compression/xprs/xprsEncoder.cpp — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/test/xprs_gtest_codec.cpp — added include, replaced 1× `WITH_NVCODEC` with `XPRS_HAS_NVENC` (test exercises NV encoders by name). arvr/projects/compression/xprs/test/xprs_gtest_common.h — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/BUCK — added `NvCodecConfig.h` to `SUPPORTED_PLATFORMS_HEADERS` so it's exposed to consumers. NOT changed (out of scope): arvr/projects/compression/xprs/CMakeLists.txt — has the existing `target_compile_options` instead of `target_compile_definitions` bug for ENABLE_NVCODEC; D2 fixes this. arvr/projects/compression/xprs/BUCK line 209 (`-DWITH_NVCODEC=1`) and the test BUCK / helpers.bzl equivalents — these are intentionally still WITH_NVCODEC since they pass the upstream flag that NvCodecConfig.h reads. arvr/projects/oatmeal/acro_conversion/test/BUCK (`-DWITH_NVCODEC=1`) — out-of-tree consumer, unchanged. Reviewed By: PiotrBrzyski Differential Revision: D103253723
…UDA error logs, add XPRS_DISABLE_HW_DECODE Summary: First diff in the GPU-accelerated H.265 decoding stack for projectaria-tools (see plan v11 at ~/gdrive/plans/2026-04-03-gpu-accelerated-h265-decoding-pat-v11.md and tech design at https://docs.google.com/document/d/1TMeLy0TqvAdlYo0IY3Qm4BrzE8i035z9MzYvGl_CzeM by Lou Yang). Cleans up two latent bugs in `xprsDecoderMaker` that prevent SW fallback on non-GPU machines, and reduces log spam users see when CUDA is unavailable. **Known limitation: NVDEC does not support small grayscale H.265 streams.** On Aria Gen2 recordings, the 200x200 eye-tracking and 512x512 SLAM cameras (encoded as grayscale H.265) produce `CUDA_ERROR_NOT_SUPPORTED` from `cuvidCreateDecoder` and fall back to the SW decoder. Only the 2016x1512 RGB camera stream uses the GPU path. The SW fallback machinery in this diff is what makes that graceful — without it, those streams would hard-fail. End-to-end on a 581 MB Aria Gen2 recording (RTX 5080, 580.159.03 driver), GPU vs CPU decode of the RGB stream measured 197.4 FPS vs 16.8 FPS (11.7x speedup, 300 frames). The aria_rerun_viewer load time on the same recording dropped from ~47s (CPU) to ~10s (GPU). Bug 1 (line 386 of arvr/libraries/vrs/utils/xprs/XprsDecoder.cpp): when an HW decoder's `init()` returns non-OK, the function returned `nullptr` instead of trying the next decoder in the preferred list. This kills the SW fallback path entirely. Fix: log at WARN and `continue` to the next decoder. Bug 2 (line 363 of the same file): when `xprs::enumDecoders()` returned non-OK, the function bailed out even though the function intentionally collects whatever decoders it managed to enumerate before any throw. On a non-GPU machine the HW decoder loop throws during CUDA init, but SW decoders were already collected. Fix: ignore the result code and only fail if the resulting list is empty. Log noise: also downgrades all `XR_LOGE` to `XR_LOGW` in `getNvCodecContext()` (cudaContextProvider.cpp) and the catch handler in `enumDecoders`/`enumDecodersByFormat` (xprsDecApi.cpp). On a non-GPU machine these fired every VRS file open even though callers handle the throw and fall back gracefully. Adds `XPRS_DISABLE_HW_DECODE` env var: setting it to any value forces CPU-only decoding by skipping all HW decoders during enumeration. Useful for deterministic results, working around GPU memory pressure, or comparing HW-vs-SW output. Read once into a static at first call, so the value is fixed for the process lifetime — runtime mutation has no effect. Same env var skip + same catch-handler downgrade is applied to both `enumDecoders` and `enumDecodersByFormat` (parallel functions with identical structure). Reviewed By: PiotrBrzyski Differential Revision: D103253728
…PRS_HAS_NVDEC/XPRS_HAS_NVENC (facebookresearch#259) Summary: This is D1b in the GPU-accelerated H.265 decoding stack for projectaria-tools (plan v11 at ~/gdrive/plans/2026-04-03-gpu-accelerated-h265-decoding-pat-v11.md, tech design at https://docs.google.com/document/d/1TMeLy0TqvAdlYo0IY3Qm4BrzE8i035z9MzYvGl_CzeM/edit by Lou Yang). Refactor only — no behavior change. Replaces 22 in-source `#ifdef WITH_NVCODEC` directives with self-documenting `XPRS_HAS_NVDEC` (decoder side) and `XPRS_HAS_NVENC` (encoder side) macros, defined by a new `NvCodecConfig.h` single-source-of-truth header. Build-system flags (`-DWITH_NVCODEC=1` in BUCK and CMake) remain as the upstream input. Why split the macro: today both halves are gated together (`WITH_NVCODEC` is set or unset for both encoder and decoder). The split prepares for build configurations that ship only one half — most importantly the OSS PyPI wheel will want NVDEC without NVENC, since PyPI users typically want decode acceleration but the encode path requires hardware NVIDIA explicitly doesn't support across all SKUs (e.g., A100 has NVDEC but no NVENC). At the call site, `#ifdef XPRS_HAS_NVDEC` makes the intent clear in a way that `#ifdef WITH_NVCODEC` does not. Files changed: NEW arvr/projects/compression/xprs/NvCodecConfig.h — defines XPRS_HAS_NVDEC + XPRS_HAS_NVENC under WITH_NVCODEC, with explanatory comment. arvr/projects/compression/xprs/Codecs.h — removed `#include "nvEncoder.h"` (its only consumers — xprsEncApi.cpp, xprsEncoder.cpp, xprs_gtest_common.h — already include nvEncoder.h directly). Adds an explanatory comment so the next reader doesn't re-add it. arvr/projects/compression/xprs/xprsDecApi.cpp — added `#include "NvCodecConfig.h"`, replaced 4× `WITH_NVCODEC` with `XPRS_HAS_NVDEC`. arvr/projects/compression/xprs/xprsDecoder.h — added include, swapped the nvDecoder.h gate to XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsDecoder.cpp — added include, consolidated two redundant adjacent `#ifdef WITH_NVCODEC` blocks (one wrapping `cudaContextProvider.h`, one wrapping `Codecs.h` — Codecs.h was unconditionally needed and didn't actually need a guard), used XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsEncApi.cpp — added include, replaced 6× `WITH_NVCODEC` with `XPRS_HAS_NVENC`, updated TODO comment to reference the new macro. arvr/projects/compression/xprs/xprsEncoder.cpp — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/test/xprs_gtest_codec.cpp — added include, replaced 1× `WITH_NVCODEC` with `XPRS_HAS_NVENC` (test exercises NV encoders by name). arvr/projects/compression/xprs/test/xprs_gtest_common.h — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/BUCK — added `NvCodecConfig.h` to `SUPPORTED_PLATFORMS_HEADERS` so it's exposed to consumers. NOT changed (out of scope): arvr/projects/compression/xprs/CMakeLists.txt — has the existing `target_compile_options` instead of `target_compile_definitions` bug for ENABLE_NVCODEC; D2 fixes this. arvr/projects/compression/xprs/BUCK line 209 (`-DWITH_NVCODEC=1`) and the test BUCK / helpers.bzl equivalents — these are intentionally still WITH_NVCODEC since they pass the upstream flag that NvCodecConfig.h reads. arvr/projects/oatmeal/acro_conversion/test/BUCK (`-DWITH_NVCODEC=1`) — out-of-tree consumer, unchanged. Reviewed By: PiotrBrzyski Differential Revision: D103253723
YLouWashU
force-pushed
the
export-D103253723
branch
from
July 30, 2026 18:29
f64fc3d to
d280c12
Compare
YLouWashU
added a commit
to YLouWashU/vrs-1
that referenced
this pull request
Jul 30, 2026
…PRS_HAS_NVDEC/XPRS_HAS_NVENC (facebookresearch#259) Summary: This is D1b in the GPU-accelerated H.265 decoding stack for projectaria-tools (plan v11 at ~/gdrive/plans/2026-04-03-gpu-accelerated-h265-decoding-pat-v11.md, tech design at https://docs.google.com/document/d/1TMeLy0TqvAdlYo0IY3Qm4BrzE8i035z9MzYvGl_CzeM/edit by Lou Yang). Refactor only — no behavior change. Replaces 22 in-source `#ifdef WITH_NVCODEC` directives with self-documenting `XPRS_HAS_NVDEC` (decoder side) and `XPRS_HAS_NVENC` (encoder side) macros, defined by a new `NvCodecConfig.h` single-source-of-truth header. Build-system flags (`-DWITH_NVCODEC=1` in BUCK and CMake) remain as the upstream input. Why split the macro: today both halves are gated together (`WITH_NVCODEC` is set or unset for both encoder and decoder). The split prepares for build configurations that ship only one half — most importantly the OSS PyPI wheel will want NVDEC without NVENC, since PyPI users typically want decode acceleration but the encode path requires hardware NVIDIA explicitly doesn't support across all SKUs (e.g., A100 has NVDEC but no NVENC). At the call site, `#ifdef XPRS_HAS_NVDEC` makes the intent clear in a way that `#ifdef WITH_NVCODEC` does not. Files changed: NEW arvr/projects/compression/xprs/NvCodecConfig.h — defines XPRS_HAS_NVDEC + XPRS_HAS_NVENC under WITH_NVCODEC, with explanatory comment. arvr/projects/compression/xprs/Codecs.h — removed `#include "nvEncoder.h"` (its only consumers — xprsEncApi.cpp, xprsEncoder.cpp, xprs_gtest_common.h — already include nvEncoder.h directly). Adds an explanatory comment so the next reader doesn't re-add it. arvr/projects/compression/xprs/xprsDecApi.cpp — added `#include "NvCodecConfig.h"`, replaced 4× `WITH_NVCODEC` with `XPRS_HAS_NVDEC`. arvr/projects/compression/xprs/xprsDecoder.h — added include, swapped the nvDecoder.h gate to XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsDecoder.cpp — added include, consolidated two redundant adjacent `#ifdef WITH_NVCODEC` blocks (one wrapping `cudaContextProvider.h`, one wrapping `Codecs.h` — Codecs.h was unconditionally needed and didn't actually need a guard), used XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsEncApi.cpp — added include, replaced 6× `WITH_NVCODEC` with `XPRS_HAS_NVENC`, updated TODO comment to reference the new macro. arvr/projects/compression/xprs/xprsEncoder.cpp — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/test/xprs_gtest_codec.cpp — added include, replaced 1× `WITH_NVCODEC` with `XPRS_HAS_NVENC` (test exercises NV encoders by name). arvr/projects/compression/xprs/test/xprs_gtest_common.h — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/BUCK — added `NvCodecConfig.h` to `SUPPORTED_PLATFORMS_HEADERS` so it's exposed to consumers. NOT changed (out of scope): arvr/projects/compression/xprs/CMakeLists.txt — has the existing `target_compile_options` instead of `target_compile_definitions` bug for ENABLE_NVCODEC; D2 fixes this. arvr/projects/compression/xprs/BUCK line 209 (`-DWITH_NVCODEC=1`) and the test BUCK / helpers.bzl equivalents — these are intentionally still WITH_NVCODEC since they pass the upstream flag that NvCodecConfig.h reads. arvr/projects/oatmeal/acro_conversion/test/BUCK (`-DWITH_NVCODEC=1`) — out-of-tree consumer, unchanged. Reviewed By: PiotrBrzyski Differential Revision: D103253723
YLouWashU
added a commit
to YLouWashU/vrs-1
that referenced
this pull request
Jul 31, 2026
…PRS_HAS_NVDEC/XPRS_HAS_NVENC (facebookresearch#259) Summary: This is D1b in the GPU-accelerated H.265 decoding stack for projectaria-tools (plan v11 at ~/gdrive/plans/2026-04-03-gpu-accelerated-h265-decoding-pat-v11.md, tech design at https://docs.google.com/document/d/1TMeLy0TqvAdlYo0IY3Qm4BrzE8i035z9MzYvGl_CzeM/edit by Lou Yang). Refactor only — no behavior change. Replaces 22 in-source `#ifdef WITH_NVCODEC` directives with self-documenting `XPRS_HAS_NVDEC` (decoder side) and `XPRS_HAS_NVENC` (encoder side) macros, defined by a new `NvCodecConfig.h` single-source-of-truth header. Build-system flags (`-DWITH_NVCODEC=1` in BUCK and CMake) remain as the upstream input. Why split the macro: today both halves are gated together (`WITH_NVCODEC` is set or unset for both encoder and decoder). The split prepares for build configurations that ship only one half — most importantly the OSS PyPI wheel will want NVDEC without NVENC, since PyPI users typically want decode acceleration but the encode path requires hardware NVIDIA explicitly doesn't support across all SKUs (e.g., A100 has NVDEC but no NVENC). At the call site, `#ifdef XPRS_HAS_NVDEC` makes the intent clear in a way that `#ifdef WITH_NVCODEC` does not. Files changed: NEW arvr/projects/compression/xprs/NvCodecConfig.h — defines XPRS_HAS_NVDEC + XPRS_HAS_NVENC under WITH_NVCODEC, with explanatory comment. arvr/projects/compression/xprs/Codecs.h — removed `#include "nvEncoder.h"` (its only consumers — xprsEncApi.cpp, xprsEncoder.cpp, xprs_gtest_common.h — already include nvEncoder.h directly). Adds an explanatory comment so the next reader doesn't re-add it. arvr/projects/compression/xprs/xprsDecApi.cpp — added `#include "NvCodecConfig.h"`, replaced 4× `WITH_NVCODEC` with `XPRS_HAS_NVDEC`. arvr/projects/compression/xprs/xprsDecoder.h — added include, swapped the nvDecoder.h gate to XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsDecoder.cpp — added include, consolidated two redundant adjacent `#ifdef WITH_NVCODEC` blocks (one wrapping `cudaContextProvider.h`, one wrapping `Codecs.h` — Codecs.h was unconditionally needed and didn't actually need a guard), used XPRS_HAS_NVDEC. arvr/projects/compression/xprs/xprsEncApi.cpp — added include, replaced 6× `WITH_NVCODEC` with `XPRS_HAS_NVENC`, updated TODO comment to reference the new macro. arvr/projects/compression/xprs/xprsEncoder.cpp — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/test/xprs_gtest_codec.cpp — added include, replaced 1× `WITH_NVCODEC` with `XPRS_HAS_NVENC` (test exercises NV encoders by name). arvr/projects/compression/xprs/test/xprs_gtest_common.h — added include, replaced 2× `WITH_NVCODEC` with `XPRS_HAS_NVENC`. arvr/projects/compression/xprs/BUCK — added `NvCodecConfig.h` to `SUPPORTED_PLATFORMS_HEADERS` so it's exposed to consumers. NOT changed (out of scope): arvr/projects/compression/xprs/CMakeLists.txt — has the existing `target_compile_options` instead of `target_compile_definitions` bug for ENABLE_NVCODEC; D2 fixes this. arvr/projects/compression/xprs/BUCK line 209 (`-DWITH_NVCODEC=1`) and the test BUCK / helpers.bzl equivalents — these are intentionally still WITH_NVCODEC since they pass the upstream flag that NvCodecConfig.h reads. arvr/projects/oatmeal/acro_conversion/test/BUCK (`-DWITH_NVCODEC=1`) — out-of-tree consumer, unchanged. Reviewed By: PiotrBrzyski Differential Revision: D103253723
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
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.
Summary:
This is D1b in the GPU-accelerated H.265 decoding stack for projectaria-tools (plan v11 at ~/gdrive/plans/2026-04-03-gpu-accelerated-h265-decoding-pat-v11.md, tech design at https://docs.google.com/document/d/1TMeLy0TqvAdlYo0IY3Qm4BrzE8i035z9MzYvGl_CzeM/edit by Lou Yang).
Refactor only — no behavior change. Replaces 22 in-source
#ifdef WITH_NVCODECdirectives with self-documentingXPRS_HAS_NVDEC(decoder side) andXPRS_HAS_NVENC(encoder side) macros, defined by a newNvCodecConfig.hsingle-source-of-truth header. Build-system flags (-DWITH_NVCODEC=1in BUCK and CMake) remain as the upstream input.Why split the macro: today both halves are gated together (
WITH_NVCODECis set or unset for both encoder and decoder). The split prepares for build configurations that ship only one half — most importantly the OSS PyPI wheel will want NVDEC without NVENC, since PyPI users typically want decode acceleration but the encode path requires hardware NVIDIA explicitly doesn't support across all SKUs (e.g., A100 has NVDEC but no NVENC). At the call site,#ifdef XPRS_HAS_NVDECmakes the intent clear in a way that#ifdef WITH_NVCODECdoes not.Files changed:
NEW arvr/projects/compression/xprs/NvCodecConfig.h — defines XPRS_HAS_NVDEC + XPRS_HAS_NVENC under WITH_NVCODEC, with explanatory comment.
arvr/projects/compression/xprs/Codecs.h — removed
#include "nvEncoder.h"(its only consumers — xprsEncApi.cpp, xprsEncoder.cpp, xprs_gtest_common.h — already include nvEncoder.h directly). Adds an explanatory comment so the next reader doesn't re-add it.arvr/projects/compression/xprs/xprsDecApi.cpp — added
#include "NvCodecConfig.h", replaced 4×WITH_NVCODECwithXPRS_HAS_NVDEC.arvr/projects/compression/xprs/xprsDecoder.h — added include, swapped the nvDecoder.h gate to XPRS_HAS_NVDEC.
arvr/projects/compression/xprs/xprsDecoder.cpp — added include, consolidated two redundant adjacent
#ifdef WITH_NVCODECblocks (one wrappingcudaContextProvider.h, one wrappingCodecs.h— Codecs.h was unconditionally needed and didn't actually need a guard), used XPRS_HAS_NVDEC.arvr/projects/compression/xprs/xprsEncApi.cpp — added include, replaced 6×
WITH_NVCODECwithXPRS_HAS_NVENC, updated TODO comment to reference the new macro.arvr/projects/compression/xprs/xprsEncoder.cpp — added include, replaced 2×
WITH_NVCODECwithXPRS_HAS_NVENC.arvr/projects/compression/xprs/test/xprs_gtest_codec.cpp — added include, replaced 1×
WITH_NVCODECwithXPRS_HAS_NVENC(test exercises NV encoders by name).arvr/projects/compression/xprs/test/xprs_gtest_common.h — added include, replaced 2×
WITH_NVCODECwithXPRS_HAS_NVENC.arvr/projects/compression/xprs/BUCK — added
NvCodecConfig.htoSUPPORTED_PLATFORMS_HEADERSso it's exposed to consumers.NOT changed (out of scope):
arvr/projects/compression/xprs/CMakeLists.txt — has the existing
target_compile_optionsinstead oftarget_compile_definitionsbug for ENABLE_NVCODEC; D2 fixes this.arvr/projects/compression/xprs/BUCK line 209 (
-DWITH_NVCODEC=1) and the test BUCK / helpers.bzl equivalents — these are intentionally still WITH_NVCODEC since they pass the upstream flag that NvCodecConfig.h reads.arvr/projects/oatmeal/acro_conversion/test/BUCK (
-DWITH_NVCODEC=1) — out-of-tree consumer, unchanged.Reviewed By: PiotrBrzyski
Differential Revision: D103253723