-
Notifications
You must be signed in to change notification settings - Fork 4
fix: handle corrupted/partial video files (closes #12) #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -424,6 +424,79 @@ export class VideoProcessor { | |||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Validate file integrity by running ffmpeg error detection pass. | ||||||||||||||||||||||||||||||
| * Detects corruption, truncation, and bitstream errors that probe alone misses. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| private async validateFileIntegrity(filePath: string): Promise<{ corrupted: boolean; fatal: boolean; errors: string[] }> { | ||||||||||||||||||||||||||||||
| const { execFile } = await import('child_process'); | ||||||||||||||||||||||||||||||
| const ffmpegPath = this.config.encoder?.ffmpeg_path || 'ffmpeg'; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return new Promise((resolve) => { | ||||||||||||||||||||||||||||||
| const proc = execFile(ffmpegPath, ['-v', 'error', '-i', filePath, '-f', 'null', '-'], { | ||||||||||||||||||||||||||||||
| timeout: 120000, | ||||||||||||||||||||||||||||||
| }, (error, _stdout, stderr) => { | ||||||||||||||||||||||||||||||
| const output = stderr || ''; | ||||||||||||||||||||||||||||||
| const errors: string[] = []; | ||||||||||||||||||||||||||||||
| let fatal = false; | ||||||||||||||||||||||||||||||
| let corrupted = false; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Fatal: file is unplayable | ||||||||||||||||||||||||||||||
| if (output.includes('moov atom not found')) { | ||||||||||||||||||||||||||||||
| errors.push('moov atom not found (truncated MP4 — likely incomplete download)'); | ||||||||||||||||||||||||||||||
| fatal = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| if (output.includes('Invalid data found when processing input') && !output.includes('NAL unit')) { | ||||||||||||||||||||||||||||||
| errors.push('Invalid data found at container level'); | ||||||||||||||||||||||||||||||
| fatal = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| if (output.includes('End of file') || output.includes('error reading header')) { | ||||||||||||||||||||||||||||||
| errors.push('File is truncated (unexpected end of file)'); | ||||||||||||||||||||||||||||||
| corrupted = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Recoverable: corruption that error-tolerant flags can handle | ||||||||||||||||||||||||||||||
| if (output.includes('Invalid NAL unit')) { | ||||||||||||||||||||||||||||||
| errors.push('Invalid NAL unit sizes (H.264 bitstream corruption)'); | ||||||||||||||||||||||||||||||
| corrupted = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| if (output.includes('partial file')) { | ||||||||||||||||||||||||||||||
| errors.push('Partial/truncated file detected'); | ||||||||||||||||||||||||||||||
| corrupted = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| if (output.includes('error while decoding') || output.includes('decode_slice_header')) { | ||||||||||||||||||||||||||||||
| errors.push('Frame decoding errors detected'); | ||||||||||||||||||||||||||||||
| corrupted = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| if (output.includes('non-existing PPS') || output.includes('non-existing SPS')) { | ||||||||||||||||||||||||||||||
| errors.push('H.264 parameter sets missing or corrupted'); | ||||||||||||||||||||||||||||||
| corrupted = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| if (output.includes('missing picture in access unit')) { | ||||||||||||||||||||||||||||||
| errors.push('Missing pictures in access units'); | ||||||||||||||||||||||||||||||
| corrupted = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Non-zero exit with no recognized patterns — flag as potentially corrupt | ||||||||||||||||||||||||||||||
| if (error && !corrupted && !fatal && errors.length === 0) { | ||||||||||||||||||||||||||||||
| const exitCode = (error as any).code; | ||||||||||||||||||||||||||||||
| if (exitCode) { | ||||||||||||||||||||||||||||||
| errors.push(`ffmpeg integrity check exited with code ${exitCode}`); | ||||||||||||||||||||||||||||||
| corrupted = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (errors.length > 0) { | ||||||||||||||||||||||||||||||
| logger.warn(`🔍 File integrity check: ${errors.join('; ')}`); | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| logger.info('✅ File integrity check passed'); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| resolve({ corrupted: corrupted || fatal, fatal, errors }); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Get bit depth from pixel format string | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
|
|
@@ -697,14 +770,22 @@ export class VideoProcessor { | |||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // 11. 🚨 CYRILLIC/UNICODE METADATA: Handle encoding issues | ||||||||||||||||||||||||||||||
| const hasUnicodeMetadata = probe.rawMetadata?.format?.tags?.title && | ||||||||||||||||||||||||||||||
| const hasUnicodeMetadata = probe.rawMetadata?.format?.tags?.title && | ||||||||||||||||||||||||||||||
| /[^\x00-\x7F]/.test(probe.rawMetadata.format.tags.title); | ||||||||||||||||||||||||||||||
| if (hasUnicodeMetadata) { | ||||||||||||||||||||||||||||||
| // Strip problematic metadata that might cause encoding failures | ||||||||||||||||||||||||||||||
| strategy.extraOptions.push('-map_metadata', '-1'); | ||||||||||||||||||||||||||||||
| reasons.push('unicode metadata detected - strip to prevent encoding issues'); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // 12. 🩹 FILE CORRUPTION: Add error-tolerant decoding flags | ||||||||||||||||||||||||||||||
| const hasCorruption = probe.issues.some(i => i.type === 'file_corruption'); | ||||||||||||||||||||||||||||||
| if (hasCorruption) { | ||||||||||||||||||||||||||||||
| strategy.inputOptions.push('-err_detect', 'ignore_err'); | ||||||||||||||||||||||||||||||
| strategy.inputOptions.push('-fflags', '+discardcorrupt+genpts'); | ||||||||||||||||||||||||||||||
| reasons.push('corruption detected - using error-tolerant decoding'); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+781
to
+787
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Input options pushed as separate elements — works but fragile. Pushing ♻️ More robust approach const hasCorruption = probe.issues.some(i => i.type === 'file_corruption');
if (hasCorruption) {
- strategy.inputOptions.push('-err_detect', 'ignore_err');
- strategy.inputOptions.push('-fflags', '+discardcorrupt+genpts');
+ strategy.inputOptions.push('-err_detect ignore_err');
+ strategy.inputOptions.push('-fflags +discardcorrupt+genpts');
reasons.push('corruption detected - using error-tolerant decoding');
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| strategy.reason = reasons.length > 0 ? reasons.join(', ') : 'standard processing'; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return strategy; | ||||||||||||||||||||||||||||||
|
|
@@ -739,13 +820,34 @@ export class VideoProcessor { | |||||||||||||||||||||||||||||
| logger.info(`📥 Downloading source video for job ${jobId}`); | ||||||||||||||||||||||||||||||
| await this.downloadVideo(job.input.uri, sourceFile); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // 🔍 NEW: Probe input file to detect format and compatibility issues | ||||||||||||||||||||||||||||||
| // 🔍 Validate file integrity before probing/encoding | ||||||||||||||||||||||||||||||
| logger.info('🔍 Checking file integrity...'); | ||||||||||||||||||||||||||||||
| const integrity = await this.validateFileIntegrity(sourceFile); | ||||||||||||||||||||||||||||||
| if (integrity.fatal) { | ||||||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||||||
| `File is fatally corrupted: ${integrity.errors.join('; ')}. ` + | ||||||||||||||||||||||||||||||
| `Cannot process this file. Source: ${job.input.uri}` | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // 🔍 Probe input file to detect format and compatibility issues | ||||||||||||||||||||||||||||||
| logger.info(`🔍 Probing input file for compatibility...`); | ||||||||||||||||||||||||||||||
| let probeResult: FileProbeResult | null = null; | ||||||||||||||||||||||||||||||
| let encodingStrategy: EncodingStrategy | null = null; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| probeResult = await this.probeInputFile(sourceFile); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Merge integrity results into probe issues | ||||||||||||||||||||||||||||||
| if (integrity.corrupted && probeResult) { | ||||||||||||||||||||||||||||||
| probeResult.issues.push({ | ||||||||||||||||||||||||||||||
| type: 'file_corruption', | ||||||||||||||||||||||||||||||
| severity: 'warning', | ||||||||||||||||||||||||||||||
| message: `File integrity issues: ${integrity.errors.join('; ')}`, | ||||||||||||||||||||||||||||||
| suggestion: 'Will use error-tolerant decoding flags', | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| encodingStrategy = this.determineEncodingStrategy(probeResult); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| logger.info(`✅ Probe complete - Strategy: ${encodingStrategy.reason}`); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -263,6 +263,14 @@ async function encodeProfile(task: EncodingTask): Promise<void> { | |
| const segmentFiles = files.filter(f => f.endsWith('.ts')); | ||
| const stats = await fs.stat(outputPath); | ||
|
|
||
| // Validate output integrity | ||
| if (segmentFiles.length === 0) { | ||
| throw new Error('Encoding produced no output segments — possible input corruption or encoding failure'); | ||
| } | ||
| if (stats.size === 0) { | ||
| throw new Error('Playlist file is empty (0 bytes) — encoding likely failed silently'); | ||
| } | ||
|
|
||
| const result = { | ||
| profile: profile.name, | ||
| path: outputPath, | ||
|
|
@@ -315,6 +323,21 @@ async function encodeProfile(task: EncodingTask): Promise<void> { | |
| } | ||
| } | ||
|
|
||
| // Corruption-related error detection | ||
| if (errorMsg.includes('Invalid data found')) { | ||
| errorDetails = 'Input file contains corrupted data. The file may be incomplete or damaged.'; | ||
| } else if (errorMsg.includes('moov atom not found')) { | ||
| errorDetails = 'MP4 container is truncated (moov atom missing). The download was likely incomplete.'; | ||
| } else if (errorMsg.includes('error while decoding') || errorMsg.includes('decode_slice_header')) { | ||
| errorDetails = 'Video frames are corrupted. Some frames could not be decoded.'; | ||
| } else if (errorMsg.includes('non-existing PPS') || errorMsg.includes('non-existing SPS')) { | ||
| errorDetails = 'H.264 parameter sets are missing or corrupted.'; | ||
| } | ||
| if (errorMsg.includes('183') || errorMsg.includes('exit code 183')) { | ||
| errorDetails = (errorDetails ? errorDetails + ' ' : '') + | ||
| 'FFmpeg exit code 183 indicates input file corruption. Try re-downloading the source.'; | ||
| } | ||
|
Comment on lines
+336
to
+339
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exit code 183 detection pattern is overly broad — risk of false positives.
🔧 Suggested fix: use more specific pattern-if (errorMsg.includes('183') || errorMsg.includes('exit code 183')) {
+if (/\b(exit\s+)?(code\s+)?183\b/i.test(errorMsg) || errorMsg.includes('exited with code 183')) {
errorDetails = (errorDetails ? errorDetails + ' ' : '') +
'FFmpeg exit code 183 indicates input file corruption. Try re-downloading the source.';
}🤖 Prompt for AI Agents |
||
|
|
||
| // Send error to main thread | ||
| sendMessage({ | ||
| type: 'error', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Timeout handling produces confusing message; 120s may be insufficient for large files.
When timeout occurs,
error.codeis'ETIMEDOUT'(string), resulting in the message "ffmpeg integrity check exited with code ETIMEDOUT" — which isn't user-friendly.Additionally, for 2+ hour videos mentioned in PR objectives, a 120-second validation timeout may be too short when reading every frame.
♻️ Suggested improvements
return new Promise((resolve) => { - const proc = execFile(ffmpegPath, ['-v', 'error', '-i', filePath, '-f', 'null', '-'], { - timeout: 120000, + execFile(ffmpegPath, ['-v', 'error', '-i', filePath, '-f', 'null', '-'], { + timeout: 300000, // 5 minutes for large files }, (error, _stdout, stderr) => { // ... existing code ... // Non-zero exit with no recognized patterns — flag as potentially corrupt - if (error && !corrupted && !fatal && errors.length === 0) { - const exitCode = (error as any).code; - if (exitCode) { - errors.push(`ffmpeg integrity check exited with code ${exitCode}`); + if (error && !corrupted && !fatal && errors.length === 0) { + const exitCode = (error as any).code; + if (exitCode === 'ETIMEDOUT') { + errors.push('Integrity check timed out — file may be too large or unreadable'); + corrupted = true; + } else if (exitCode) { + errors.push(`ffmpeg integrity check exited with code ${exitCode}`); corrupted = true; } }Also applies to: 480-487
🤖 Prompt for AI Agents