Plugins: - misc_fixes v2.8: Pre-processing, container remux, stream conforming - stream_organizer v4.8: English priority, subtitle extraction, SRT conversion - combined_audio_standardizer v1.13: AAC/Opus encoding, downmix creation - av1_svt_converter v2.22: AV1 video encoding via SVT-AV1 Structure: - Local/ - Plugin .js files (mount in Tdarr) - agent_notes/ - Development documentation - Latest-Reports/ - Error logs for analysis
265 lines
7.6 KiB
Markdown
265 lines
7.6 KiB
Markdown
# Misc Fixes Plugin - Complete Analysis
|
||
|
||
**Version**: 2.1
|
||
**File**: `Tdarr_Plugin_misc_fixes.js`
|
||
**Lines**: 239
|
||
**Syntax**: ✅ Valid
|
||
|
||
---
|
||
|
||
## Executive Summary
|
||
|
||
The plugin is **functionally sound** with no critical bugs. Syntax is valid and the core logic correctly implements all Megamix features. Found **5 improvements**, **3 edge cases**, and **2 minor redundancies**.
|
||
|
||
---
|
||
|
||
## 🟢 Strengths
|
||
|
||
1. **Robust Input Validation**: Validates all inputs with clear error messages
|
||
2. **Star Marker Stripping**: Properly handles UI default indicators
|
||
3. **Clear Logic Flow**: Well-commented sections for each feature
|
||
4. **Proper Stream Handling**: Correctly iterates and identifies streams
|
||
5. **Flexible Mapping**: Uses negative mapping for precise stream exclusion
|
||
6. **Container Awareness**: Different logic for MKV vs MP4 targets
|
||
|
||
---
|
||
|
||
## 🟡 Edge Cases Identified
|
||
|
||
### 1. **Video Stream Existence Not Validated**
|
||
**Severity**: Medium
|
||
**Location**: Lines 163-197 (stream loop)
|
||
|
||
**Issue**: Plugin doesn't check if video streams exist before processing.
|
||
|
||
**Scenario**:
|
||
- Audio-only file (music, audiobook)
|
||
- File with only subtitle/data streams
|
||
|
||
**Impact**: Plugin will process but may produce unexpected results for non-video files.
|
||
|
||
**Fix**: Add video stream check before processing:
|
||
```javascript
|
||
const hasVideo = file.ffProbeData.streams.some(s => s.codec_type === 'video');
|
||
if (!hasVideo) {
|
||
response.infoLog += '⚠️ No video stream found, skipping. ';
|
||
return response;
|
||
}
|
||
```
|
||
|
||
### 2. **Stream Reorder Logic Inconsistency**
|
||
**Severity**: Low
|
||
**Location**: Lines 155-161
|
||
|
||
**Issue**: When `ensure_video_first === 'true'`, the plugin sets `baseMap` to explicit order but doesn't set `needsRemux = true` unconditionally.
|
||
|
||
**Scenario**:
|
||
- Container matches target
|
||
- No streams dropped
|
||
- Video is already first
|
||
- `ensure_video_first` is enabled
|
||
|
||
**Impact**: Plugin will use explicit mapping but won't actually process the file unless another condition triggers remux.
|
||
|
||
**Current Behavior**: Only triggers if stream 0 is NOT video (line 203).
|
||
|
||
**Recommendation**: This is actually **correct behavior** - it only reorders if needed. The comment on line 160 is slightly misleading though.
|
||
|
||
### 3. **Empty Streams Array**
|
||
**Severity**: Low
|
||
**Location**: Line 164
|
||
|
||
**Issue**: Loop assumes `file.ffProbeData.streams.length > 0`.
|
||
|
||
**Scenario**: File has `streams: []` (empty array).
|
||
|
||
**Impact**: Loop won't execute, plugin will return "File meets all criteria" even for problematic files.
|
||
|
||
**Fix**: Already protected by line 118 check for array existence. Could add:
|
||
```javascript
|
||
if (file.ffProbeData.streams.length === 0) {
|
||
response.infoLog += '❌ No streams found in file. ';
|
||
return response;
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
## 🔵 Improvements Suggested
|
||
|
||
### 1. **Add File Medium Check**
|
||
**Priority**: High
|
||
**Rationale**: Migz plugins check `file.fileMedium !== 'video'`
|
||
|
||
**Add after input validation (line 121)**:
|
||
```javascript
|
||
if (file.fileMedium !== 'video') {
|
||
response.infoLog += '⚠️ File is not a video. ';
|
||
return response;
|
||
}
|
||
```
|
||
|
||
### 2. **Duplicate Stream Dropping**
|
||
**Priority**: Medium
|
||
**Location**: Lines 169-196
|
||
|
||
**Issue**: If a stream matches both image removal AND conform rules, it's added to `extraMaps` twice.
|
||
|
||
**Example**: A `data` stream in a file targeting MKV would be dropped by both:
|
||
- Line 182: `type === 'data'`
|
||
- Potentially line 172: If it's a video type with mjpeg/png/gif codec
|
||
|
||
**Impact**: Duplicate `-map -0:X` flags (harmless but inefficient).
|
||
|
||
**Fix**: Track dropped streams:
|
||
```javascript
|
||
const droppedStreams = new Set();
|
||
|
||
// In loop:
|
||
if (!droppedStreams.has(i)) {
|
||
extraMaps.push(`-map -0:${i}`);
|
||
droppedStreams.add(i);
|
||
response.infoLog += `ℹ️ Removing stream ${i} (${codec}). `;
|
||
droppingStreams = true;
|
||
}
|
||
```
|
||
|
||
### 3. **WebM Container Support**
|
||
**Priority**: Low
|
||
**Benefit**: WebM is a common modern container
|
||
|
||
**Add to inputs**:
|
||
```javascript
|
||
options: ['mkv', 'mp4', 'webm'],
|
||
```
|
||
|
||
**Add conform rules** (after line 195):
|
||
```javascript
|
||
} else if (targetContainer === 'webm') {
|
||
// WebM only supports VP8/VP9/AV1 video, Opus/Vorbis audio
|
||
// Drop incompatible streams
|
||
if (['mov_text', 'eia_608', 'timed_id3', 'hdmv_pgs_subtitle', 'subrip'].includes(codec)) {
|
||
extraMaps.push(`-map -0:${i}`);
|
||
response.infoLog += `ℹ️ Dropping incompatible stream ${i} (${codec}) for WebM. `;
|
||
droppingStreams = true;
|
||
}
|
||
}
|
||
```
|
||
|
||
### 4. **Default Container Mismatch**
|
||
**Priority**: Low
|
||
**Location**: Line 91
|
||
|
||
**Issue**: Sets `response.container = .${file.container}` initially, then changes to target on line 222.
|
||
|
||
**Improvement**: Only set if not processing:
|
||
```javascript
|
||
// At line 233, before final return:
|
||
response.container = `.${file.container}`; // Restore original
|
||
```
|
||
|
||
Actually, this is fine - line 222 overwrites it when processing. No change needed.
|
||
|
||
### 5. **Log Consolidation**
|
||
**Priority**: Low
|
||
**Benefit**: Cleaner logs
|
||
|
||
**Current**: Logs each dropped stream individually.
|
||
**Improvement**: Consolidate:
|
||
```javascript
|
||
const droppedStreamDetails = [];
|
||
// In loop, collect instead of logging immediately:
|
||
droppedStreamDetails.push(`${i}:${codec}`);
|
||
|
||
// After loop:
|
||
if (droppedStreamDetails.length > 0) {
|
||
response.infoLog += `ℹ️ Dropping streams: ${droppedStreamDetails.join(', ')}. `;
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
## 🟠 Redundancies Found
|
||
|
||
### 1. **Boolean Validation Redundancy**
|
||
**Location**: Lines 107-116
|
||
|
||
**Issue**: `booleanInputs` array is defined but only used in one place.
|
||
|
||
**Current**:
|
||
```javascript
|
||
const booleanInputs = ['force_conform', 'remove_image_streams', 'ensure_video_first', 'fix_ts_timestamps'];
|
||
for (const input of booleanInputs) {
|
||
// validate
|
||
}
|
||
```
|
||
|
||
**Optimization**: Since we're looping through known keys, this is fine. Not truly redundant. ✅
|
||
|
||
### 2. **Container Check Redundancy**
|
||
**Location**: Lines 126-127
|
||
|
||
**Issue**: Both `isTargetMkv` and `isTargetMp4` are set, but only one can be true.
|
||
|
||
**Purpose**: Readability - makes conditionals clearer.
|
||
**Verdict**: Keep as-is for clarity. ✅
|
||
|
||
---
|
||
|
||
## 🔴 Bugs Found
|
||
|
||
### None Identified ✅
|
||
|
||
All logic branches are correct. No syntax errors, no runtime issues expected.
|
||
|
||
---
|
||
|
||
## 📊 Code Quality Metrics
|
||
|
||
| Metric | Score | Notes |
|
||
|--------|-------|-------|
|
||
| Syntax Validity | ✅ 100% | Passes Node.js check |
|
||
| Input Validation | ✅ Excellent | All inputs validated |
|
||
| Error Handling | ✅ Good | Returns early on errors |
|
||
| Code Clarity | ✅ Excellent | Well-commented |
|
||
| Performance | ✅ Good | Single-pass stream iteration |
|
||
| Edge Case Handling | 🟡 Good | Missing video check |
|
||
|
||
---
|
||
|
||
## 🎯 Prioritized Recommendations
|
||
|
||
### Must Implement
|
||
1. ✅ **Add file medium check** (prevents non-video processing)
|
||
2. ✅ **Add video stream existence check** (prevents audio-only processing)
|
||
|
||
### Should Implement
|
||
3. 🟡 **Prevent duplicate stream drops** (minor efficiency gain)
|
||
|
||
### Nice to Have
|
||
4. 🔵 **Add WebM container support** (modernizes plugin)
|
||
5. 🔵 **Consolidate stream drop logs** (cleaner output)
|
||
|
||
---
|
||
|
||
## 🧪 Test Scenarios
|
||
|
||
### Recommended Test Cases
|
||
|
||
1. **TS file with broken timestamps** → Should apply genpts flags
|
||
2. **MKV file with mov_text subs** → Should drop subs
|
||
3. **MP4 with PGS subtitles** → Should drop subs
|
||
4. **Audio-only file** → Should skip (after adding check)
|
||
5. **File with video NOT first** → Should reorder
|
||
6. **File with MJPEG cover art** → Should remove
|
||
7. **File already matching all criteria** → Should skip processing
|
||
8. **Empty streams array** → Should error gracefully
|
||
|
||
---
|
||
|
||
## 📝 Summary
|
||
|
||
The plugin is **production-ready** with minor improvements recommended. No critical bugs found. The logic is sound and follows best practices from the original Migz/Lmg1 plugins.
|
||
|
||
**Final Grade**: A- (would be A+ with video stream check)
|