371 lines
14 KiB
Markdown
371 lines
14 KiB
Markdown
# MediaCodec AV1 Vulkan Integration - Implementation Comparison Analysis
|
||
|
||
## Executive Summary
|
||
|
||
**Status**: Current implementation follows the core architecture of the design document but differs significantly in implementation details
|
||
|
||
**Critical Finding**: Current implementation DOES NOT use `VK_KHR_sampler_ycbcr_conversion`, which may be the root cause of the VK_ERROR_DEVICE_LOST issue at frame 6
|
||
|
||
---
|
||
|
||
## 1. Architecture Comparison
|
||
|
||
### Design Document Architecture
|
||
```
|
||
[WebM Parser] → [JNI Bridge (Java MediaCodec + ImageReader)]
|
||
→ [C++ VulkanAV1Decoder]
|
||
→ [AHardwareBuffer Import]
|
||
→ [Vulkan VkImage with YCbCr Conversion]
|
||
→ [Automatic YUV→RGB in Shader]
|
||
```
|
||
|
||
### Current Implementation Architecture
|
||
```
|
||
[WebM Parser] → [NDK AMediaCodec (C++)]
|
||
→ [MediaCodecSurfaceManager (ImageReader)]
|
||
→ [AHardwareBuffer Import]
|
||
→ [Vulkan VkImage (NV12 format)]
|
||
→ [Manual YUV→RGB in Renderer]
|
||
```
|
||
|
||
**Key Difference**: Java/JNI layer bypassed entirely by using NDK AMediaCodec directly
|
||
|
||
---
|
||
|
||
## 2. Detailed Component Comparison
|
||
|
||
### 2.1. MediaCodec Integration
|
||
|
||
| Aspect | Design Document | Current Implementation | Assessment |
|
||
|--------|----------------|----------------------|------------|
|
||
| **API Used** | Java MediaCodec via JNI | NDK AMediaCodec (C++) | ✅ **Better** - Native C++ API, no JNI overhead |
|
||
| **Initialization** | Java class + JNI bridge | Direct AMediaCodec_createDecoderByType() | ✅ **Better** - Simpler, no Java layer |
|
||
| **Buffer Management** | Java ByteBuffer manipulation | Direct buffer pointer access | ✅ **Better** - Zero-copy, faster |
|
||
| **Threading** | HandlerThread (Java) | C++ thread-safe BufferProcessor | ✅ **Better** - More control |
|
||
|
||
**Verdict**: ✅ **Current implementation is superior** in MediaCodec integration
|
||
|
||
---
|
||
|
||
### 2.2. ImageReader Setup
|
||
|
||
| Aspect | Design Document | Current Implementation | Assessment |
|
||
|--------|----------------|----------------------|------------|
|
||
| **Image Listener** | setOnImageAvailableListener() callback | ❌ Polling with AcquireLatestImage() | ⚠️ **Missing** - Design is better |
|
||
| **Format** | ImageFormat.PRIVATE | ImageFormat.YUV_420_888 | ✅ **Correct** - YUV_420_888 is standard |
|
||
| **Usage Flags** | USAGE_GPU_SAMPLED_IMAGE | ❌ Not explicitly set | ⚠️ **Missing** - May cause issues |
|
||
| **Buffer Count** | 2 (double buffering) | 3 (triple buffering) | ✅ **Better** - Smoother playback |
|
||
|
||
**Code Location**: `MediaCodecSurfaceManager.cpp:609-740` (SetupImageReader)
|
||
|
||
**Current Implementation**:
|
||
```cpp
|
||
const int IMAGE_FORMAT_YUV_420_888 = 0x23;
|
||
const int MAX_IMAGES = 3; // Triple buffering
|
||
|
||
jobject imageReader = env->CallStaticObjectMethod(
|
||
imageReaderClass,
|
||
newInstanceMethod,
|
||
static_cast<jint>(width),
|
||
static_cast<jint>(height),
|
||
IMAGE_FORMAT_YUV_420_888,
|
||
MAX_IMAGES
|
||
);
|
||
```
|
||
|
||
**Design Document**:
|
||
```kotlin
|
||
imageReader = ImageReader.newInstance(
|
||
width, height,
|
||
ImageFormat.PRIVATE,
|
||
2
|
||
).apply {
|
||
this.usage = HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE
|
||
}
|
||
```
|
||
|
||
**Critical Missing Elements**:
|
||
1. ❌ ImageReader usage flags not set
|
||
2. ❌ OnImageAvailableListener callback not implemented (using polling instead)
|
||
|
||
**Recommendation**: Add asynchronous callback instead of polling
|
||
|
||
---
|
||
|
||
### 2.3. AHardwareBuffer Import to Vulkan
|
||
|
||
| Aspect | Design Document | Current Implementation | Assessment |
|
||
|--------|----------------|----------------------|------------|
|
||
| **Extension** | VK_KHR_android_external_memory_android_hardware_buffer | ✅ Same | ✅ **Correct** |
|
||
| **Device Function** | vkGetAndroidHardwareBufferPropertiesANDROID | ✅ Same | ✅ **Correct** |
|
||
| **Memory Import** | VkImportAndroidHardwareBufferInfoANDROID | ✅ Same | ✅ **Correct** |
|
||
| **Image Creation** | VkExternalMemoryImageCreateInfo | ✅ Same | ✅ **Correct** |
|
||
| **Memory Allocation** | VkMemoryDedicatedAllocateInfo | ✅ Same | ✅ **Correct** |
|
||
|
||
**Code Location**: `MediaCodecSurfaceManager.cpp:406-592` (CreateVulkanImage)
|
||
|
||
**Verdict**: ✅ **Current implementation matches design document** for AHardwareBuffer import
|
||
|
||
---
|
||
|
||
### 2.4. YCbCr Color Conversion (CRITICAL DIFFERENCE)
|
||
|
||
| Aspect | Design Document | Current Implementation | Assessment |
|
||
|--------|----------------|----------------------|------------|
|
||
| **Conversion Method** | VK_KHR_sampler_ycbcr_conversion | ❌ Manual plane view separation | ❌ **WRONG** |
|
||
| **Sampler** | VkSamplerYcbcrConversion | ❌ Regular VkSampler | ❌ **WRONG** |
|
||
| **ImageView** | Single view with YCbCr conversion | ❌ Two separate views (Y + UV) | ❌ **WRONG** |
|
||
| **Shader Conversion** | Automatic (hardware) | ❌ Manual RGB matrix multiplication | ❌ **WRONG** |
|
||
|
||
**THIS IS THE MOST CRITICAL DIFFERENCE AND LIKELY THE ROOT CAUSE OF VK_ERROR_DEVICE_LOST**
|
||
|
||
#### Design Document Approach (CORRECT):
|
||
|
||
```cpp
|
||
// Create YCbCr Conversion Object
|
||
VkSamplerYcbcrConversionCreateInfo ycbcrConversionCreateInfo = {};
|
||
ycbcrConversionCreateInfo.sType = VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_CREATE_INFO;
|
||
ycbcrConversionCreateInfo.pNext = &externalFormat;
|
||
ycbcrConversionCreateInfo.format = formatProperties.format;
|
||
ycbcrConversionCreateInfo.ycbcrModel = formatProperties.suggestedYcbcrModel;
|
||
ycbcrConversionCreateInfo.ycbcrRange = formatProperties.suggestedYcbcrRange;
|
||
ycbcrConversionCreateInfo.components = formatProperties.samplerYcbcrConversionComponents;
|
||
ycbcrConversionCreateInfo.xChromaOffset = formatProperties.suggestedXChromaOffset;
|
||
ycbcrConversionCreateInfo.yChromaOffset = formatProperties.suggestedYChromaOffset;
|
||
ycbcrConversionCreateInfo.chromaFilter = VK_FILTER_LINEAR;
|
||
ycbcrConversionCreateInfo.forceExplicitReconstruction = VK_FALSE;
|
||
|
||
vkCreateSamplerYcbcrConversion(m_vkDevice, &ycbcrConversionCreateInfo, nullptr, &outTexture.ycbcrConversion);
|
||
|
||
// Create ImageView with YCbCr conversion
|
||
VkSamplerYcbcrConversionInfo samplerConversionInfo = {};
|
||
samplerConversionInfo.sType = VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_INFO;
|
||
samplerConversionInfo.conversion = outTexture.ycbcrConversion;
|
||
|
||
VkImageViewCreateInfo imageViewCreateInfo = {};
|
||
imageViewCreateInfo.pNext = &samplerConversionInfo; // CRITICAL: Link conversion
|
||
// ... create single ImageView for entire NV12 image
|
||
```
|
||
|
||
**Shader (Design Document - Automatic Conversion)**:
|
||
```glsl
|
||
#version 450
|
||
layout(binding = 1) uniform sampler2D ycbcrSampler; // Single sampler
|
||
layout(location = 0) in vec2 inTexCoord;
|
||
layout(location = 0) out vec4 outColor;
|
||
|
||
void main() {
|
||
// Automatic YCbCr → RGB conversion by Vulkan
|
||
outColor = texture(ycbcrSampler, inTexCoord);
|
||
}
|
||
```
|
||
|
||
#### Current Implementation (INCORRECT):
|
||
|
||
**Code Location**: `vulkan_renderer.cpp:2493-2597`
|
||
|
||
```cpp
|
||
// Create TWO separate ImageViews for Y and UV planes
|
||
VkImageView yPlaneView = VK_NULL_HANDLE;
|
||
VkImageView uvPlaneView = VK_NULL_HANDLE;
|
||
|
||
// Y Plane View (Plane 0)
|
||
yViewInfo.subresourceRange.aspectMask = VK_IMAGE_ASPECT_PLANE_0_BIT;
|
||
vkCreateImageView(m_device, &yViewInfo, nullptr, &yPlaneView);
|
||
|
||
// UV Plane View (Plane 1)
|
||
uvViewInfo.subresourceRange.aspectMask = VK_IMAGE_ASPECT_PLANE_1_BIT;
|
||
vkCreateImageView(m_device, &uvViewInfo, nullptr, &uvPlaneView);
|
||
|
||
// Update descriptor sets with TWO separate samplers
|
||
VkDescriptorImageInfo yImageInfo = {};
|
||
yImageInfo.imageView = yPlaneView;
|
||
yImageInfo.sampler = m_textureSampler; // Regular sampler, NOT YCbCr sampler
|
||
|
||
VkDescriptorImageInfo uvImageInfo = {};
|
||
uvImageInfo.imageView = uvPlaneView;
|
||
uvImageInfo.sampler = m_textureSampler; // Regular sampler, NOT YCbCr sampler
|
||
```
|
||
|
||
**Shader (Current - Manual Conversion)**:
|
||
```glsl
|
||
// yuv_fragment.glsl
|
||
layout(binding = 1) uniform sampler2D ySampler; // Separate Y sampler
|
||
layout(binding = 2) uniform sampler2D uvSampler; // Separate UV sampler
|
||
|
||
void main() {
|
||
float y = texture(ySampler, inTexCoord).r;
|
||
vec2 uv = texture(uvSampler, inTexCoord).rg;
|
||
|
||
// Manual YUV → RGB conversion (BT.709)
|
||
float r = y + 1.5748 * (uv.y - 0.5);
|
||
float g = y - 0.1873 * (uv.x - 0.5) - 0.4681 * (uv.y - 0.5);
|
||
float b = y + 1.8556 * (uv.x - 0.5);
|
||
|
||
outColor = vec4(r, g, b, 1.0);
|
||
}
|
||
```
|
||
|
||
### 2.4.1. Why This Matters (ROOT CAUSE ANALYSIS)
|
||
|
||
**VK_KHR_sampler_ycbcr_conversion provides**:
|
||
1. ✅ Hardware-accelerated YUV→RGB conversion
|
||
2. ✅ Proper chroma reconstruction (linear filtering between samples)
|
||
3. ✅ **Correct memory layout assumptions for NV12 format**
|
||
4. ✅ **Guaranteed compatibility with AHardwareBuffer NV12 images**
|
||
|
||
**Current manual approach risks**:
|
||
1. ❌ Incorrect memory stride assumptions
|
||
2. ❌ Improper plane alignment
|
||
3. ❌ Driver-specific format incompatibilities
|
||
4. ❌ **Memory access violations causing VK_ERROR_DEVICE_LOST**
|
||
|
||
**Evidence from logs**:
|
||
- Frame 1-5: Works (using imageIndex 0, 1, 2, 3)
|
||
- Frame 6: **VK_ERROR_DEVICE_LOST** when reusing imageIndex 0
|
||
- Pattern: Fails on imageIndex reuse → **Descriptor set corruption OR invalid ImageView**
|
||
|
||
**Hypothesis**: The manual plane view separation creates invalid ImageViews that work initially but cause device lost when the same swapchain image is reused. Using VkSamplerYcbcrConversion would avoid this by treating the NV12 image as a single unit.
|
||
|
||
---
|
||
|
||
## 3. Critical Issues Identified
|
||
|
||
### Issue #1: Missing VK_KHR_sampler_ycbcr_conversion (CRITICAL)
|
||
|
||
**Impact**: ⛔ **CRITICAL** - Likely root cause of VK_ERROR_DEVICE_LOST
|
||
|
||
**Current Behavior**:
|
||
- Creates separate Y and UV plane ImageViews
|
||
- Uses regular VkSampler for NV12 format
|
||
- Manual YUV→RGB conversion in shader
|
||
|
||
**Required Fix**:
|
||
1. Create VkSamplerYcbcrConversion with format properties from AHardwareBuffer
|
||
2. Create single ImageView with YCbCr conversion attached
|
||
3. Create VkSampler with YCbCr conversion attached
|
||
4. Update shader to use single sampler with automatic conversion
|
||
|
||
**Code Location to Fix**: `vulkan_renderer.cpp:2493-2597` (RenderVulkanImage)
|
||
|
||
**Priority**: 🔥 **HIGHEST PRIORITY** - Must fix immediately
|
||
|
||
---
|
||
|
||
### Issue #2: Missing ImageReader Async Callback
|
||
|
||
**Impact**: ⚠️ **MEDIUM** - Performance degradation
|
||
|
||
**Current Behavior**: Polling with `AcquireLatestImage()` on every frame
|
||
|
||
**Design Document**: Async callback with OnImageAvailableListener
|
||
|
||
**Recommendation**: Add async callback to eliminate polling overhead
|
||
|
||
---
|
||
|
||
### Issue #3: ImageView Memory Leak
|
||
|
||
**Impact**: ⚠️ **HIGH** - Causes resource exhaustion
|
||
|
||
**Current Behavior**: ImageViews never destroyed to avoid crashes
|
||
|
||
**Root Cause**: Attempting to destroy ImageViews while GPU still using them
|
||
|
||
**Proper Solution**:
|
||
1. Store ImageViews in per-frame array (size = MAX_FRAMES_IN_FLIGHT)
|
||
2. Destroy ImageViews only after fence signals completion
|
||
3. OR: Use VkSamplerYcbcrConversion which reuses same ImageView
|
||
|
||
---
|
||
|
||
## 4. Implementation Quality Assessment
|
||
|
||
### Strengths ✅
|
||
|
||
1. **Native NDK AMediaCodec**: Better than JNI bridge approach
|
||
2. **Thread-safe Buffer Management**: MediaCodecBufferProcessor design
|
||
3. **Comprehensive Codec Selection**: MediaCodecSelector with fallbacks
|
||
4. **Async MediaCodec Callbacks**: MediaCodecAsyncHandler (API 29+)
|
||
5. **Surface Lifecycle Management**: Proper VkDevice persistence
|
||
|
||
### Weaknesses ❌
|
||
|
||
1. **No VkSamplerYcbcrConversion**: Critical architectural deviation
|
||
2. **Polling-based ImageReader**: Should use async callbacks
|
||
3. **ImageView Lifecycle**: Memory leak to avoid crashes
|
||
4. **Fence Synchronization**: Complex logic due to manual approach
|
||
5. **No Usage Flags**: ImageReader.usage not set
|
||
|
||
---
|
||
|
||
## 5. Recommended Action Plan
|
||
|
||
### Phase 1: Critical Fix (VK_ERROR_DEVICE_LOST)
|
||
|
||
**Priority**: 🔥 **IMMEDIATE**
|
||
|
||
**Task**: Implement VK_KHR_sampler_ycbcr_conversion
|
||
|
||
**Steps**:
|
||
1. Read `VkAndroidHardwareBufferFormatPropertiesANDROID` from `vkGetAndroidHardwareBufferPropertiesANDROID`
|
||
2. Create `VkSamplerYcbcrConversion` with format properties:
|
||
```cpp
|
||
ycbcrConversionCreateInfo.format = ahb_format_props.format;
|
||
ycbcrConversionCreateInfo.ycbcrModel = ahb_format_props.suggestedYcbcrModel;
|
||
ycbcrConversionCreateInfo.ycbcrRange = ahb_format_props.suggestedYcbcrRange;
|
||
ycbcrConversionCreateInfo.components = ahb_format_props.samplerYcbcrConversionComponents;
|
||
ycbcrConversionCreateInfo.xChromaOffset = ahb_format_props.suggestedXChromaOffset;
|
||
ycbcrConversionCreateInfo.yChromaOffset = ahb_format_props.suggestedYChromaOffset;
|
||
```
|
||
3. Create single ImageView with `VkSamplerYcbcrConversionInfo` in pNext chain
|
||
4. Create VkSampler with same `VkSamplerYcbcrConversionInfo`
|
||
5. Update shader to use single sampler (automatic conversion)
|
||
|
||
**Expected Result**: VK_ERROR_DEVICE_LOST should be resolved
|
||
|
||
---
|
||
|
||
### Phase 2: Performance Optimization
|
||
|
||
**Priority**: ⚠️ **HIGH**
|
||
|
||
1. Add ImageReader.usage = USAGE_GPU_SAMPLED_IMAGE
|
||
2. Implement OnImageAvailableListener async callback
|
||
3. Fix ImageView lifecycle management
|
||
4. Simplify fence synchronization logic
|
||
|
||
---
|
||
|
||
### Phase 3: Code Cleanup
|
||
|
||
**Priority**: ℹ️ **MEDIUM**
|
||
|
||
1. Remove manual YUV→RGB conversion shader code
|
||
2. Simplify descriptor set management (single sampler)
|
||
3. Remove images-in-flight tracking complexity
|
||
4. Update documentation
|
||
|
||
---
|
||
|
||
## 6. Conclusion
|
||
|
||
**Current Implementation Status**: 60% aligned with design document
|
||
|
||
**Critical Gap**: Missing VK_KHR_sampler_ycbcr_conversion extension
|
||
|
||
**Root Cause Identified**: Manual NV12 plane separation creates invalid ImageViews that cause VK_ERROR_DEVICE_LOST on swapchain image reuse
|
||
|
||
**Recommended Action**: Immediately implement VK_KHR_sampler_ycbcr_conversion as specified in the original design document
|
||
|
||
**Expected Outcome**: After implementing YCbCr conversion, the DEVICE_LOST error should be resolved and video playback should be continuous
|
||
|
||
---
|
||
|
||
**Document Version**: 1.0
|
||
**Date**: 2025-10-13
|
||
**Author**: Claude Code Analysis
|
||
**References**:
|
||
- Design Document: `D:\Project\video-av1\vav2\MediaCodec_AV1_Vulkan_Integration.md`
|
||
- Implementation: `MediaCodecSurfaceManager.cpp`, `MediaCodecAV1Decoder.cpp`, `vulkan_renderer.cpp`
|