#7064: Check integrity of MPEG-TS video stream.

[mstyura]

This PR will...

Add integrity checker of MPEG-TS stream for video and allow skipping broken data.

Why is this Pull Request needed?

Currently MPEG-TS data corruption is not checked and hence broken data reaches video decoder, causing it to fail, which require manual error recovery.

Are there any points in the code the reviewer needs to double check?

The following piece of FFMpeg was used as an inspiration to checks: https://github.com/FFmpeg/FFmpeg/blob/e4c8e80a2efee275f2a10fcf0424c9fc1d86e309/libavformat/mpegts.c#L2811-L2834

Resolves issues:

#7064 - the playback of stream attached into the issue does not cause decoder errors anymore;

The behaviour of Safari's HLS player on corrupted stream:
https://github.com/user-attachments/assets/c848b1e0-0f10-4f5d-87ee-33dc5ded0c6c

The behaviour of HLS.js with skip strategy of corrupted video:
https://github.com/user-attachments/assets/544339e9-013e-443d-ba59-39864bca5ef2

Checklist

• changes have been done against master branch, and PR does not conflict
• new unit / functional tests have been added (whenever applicable)
• API or design changes are documented in API.md

[mstyura]

Hello @robwalch, could you please take a look at this PR once you have a time? Thanks you very much in advance!

[robwalch]

Would this also resolve #6912? More specifically, would it detect corruption in the sample provided in the description?

I appreciate that the integrity can be enabled and disabled and the default matches current behavior. I wonder though if this should just be built into parsePES with handlers for continuity issues and tie flag. I suppose that optimization could be made later.

My biggest concern is whether simply skipping packets is enough. A parsing error should be raised when enough media is corrupt that either the parsed data should not be used or an alternate should be selected.

[mstyura]

Would this also resolve #6912? More specifically, would it detect corruption in the sample provided in the description?

I've just checked, and it seems that the provided algorithm doesn't detect corruption in the stream attached to the linked issue. The nature of the issue appears to be slightly different from what I've encountered, even though both lead to a decoder error. The issue I’m fixing is observable via ffplay -v debug as:

[mpegts @ 0x13e01e540] Continuity check failed for pid 256 expected 1 got 2
[mpegts @ 0x13e01e540] Packet corrupt (stream = 0, dts = 82320210).
[mpegts @ 0x13e01e540] Continuity check failed for pid 256 expected 9 got 10
[mpegts @ 0x13e01e540] Packet corrupt (stream = 0, dts = 82323270).
[mpegts @ 0x13e01e540] Continuity check failed for pid 256 expected 1 got 2
[mpegts @ 0x13e01e540] Packet corrupt (stream = 0, dts = 82326330).
[mpegts @ 0x13e01e540] Continuity check failed for pid 256 expected 9 got 10

and the solution is to skip samples with missing parts, thereby avoiding the decoder error in favor of smoother playback, though the resulting video may have artifacts until the next key frame. From the attached issue, it seems the problem lies in the fragment 00113.ts, for which ffplay produces the following warning:

[hevc @ 0x14c67dbc0] Decoded frame with POC 0/12.
[hevc @ 0x14c667e40] Could not find ref with POC 2
[hevc @ 0x14c667e40] Output frame with POC 0/3.KB sq=    0B 

which seems to be an issue with origin encoder.

My biggest concern is whether simply skipping packets is enough.

As far as I understand, my change skips the entire sample if any part of it is missing.

A parsing error should be raised when enough media is corrupt that either the parsed data should not be used or an alternate should be selected.

I initially thought I might have missed an event that could be raised here, so I really appreciate the maintainer's suggestion. From my side, the only concern is whether it’s worth switching to an alternative fragment in the case of a single or a few corrupted samples within a segment. Ideally, the backend should never publish corrupted fragments, but unfortunately, this may still happen, resulting in a fragment where individual video samples are corrupted.

[mstyura]

Probably this might also fix the issue mentioned here: #4506 (comment) because I reproduced it in exactly same scenario - TS data received over UDP was lost/reordered, but was packed to hls as is causing decoder errors.

[robwalch]

@mstyura, shouldn't this be repeated in extractRemainingSamples where additional PES parsing is performed?

[mstyura]

Yes, that makes sense. Should I update the PR (but tomorrow), or will you take it? I see you've already back-merged recent changes - that's why I'm asking.

[robwalch]

Please make the change if you don't mind. I noticed the issue while rebasing in github. Thanks for being patient with this one.

[mstyura]

Sorry it took longer than I promised, but now it's rebased and updated.

[robwalch]

@mstyura Why only check the integrity of video PES packets?

Would it be better or even simpler to check the integrity of all packets? I guess you would need one instance per PID then.

[mstyura]

I had the problem with video not being able to decode due to a missing I-frame within GOP, so I've added check for video. I'm not sure if audio had same kind of inter-frame dependency, but I didn't experience any issue decoding audio. So, simply put, I was fixing the problem I know definitely exists and able to reproduce. I don't know for sure if such check is worth doing of audio.