-
Notifications
You must be signed in to change notification settings - Fork 2.7k
#7064: Check integrity of MPEG-TS video stream. #7094
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: master
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 |
---|---|---|
|
@@ -72,6 +72,7 @@ class TSDemuxer implements Demuxer { | |
private aacOverFlow: AudioFrame | null = null; | ||
private remainderData: Uint8Array | null = null; | ||
private videoParser: BaseVideoParser | null; | ||
private videoIntegrityChecker: PacketsIntegrityChecker | null = null; | ||
|
||
constructor( | ||
observer: HlsEventEmitter, | ||
|
@@ -182,6 +183,10 @@ class TSDemuxer implements Demuxer { | |
|
||
this._videoTrack = TSDemuxer.createTrack('video') as DemuxedVideoTrack; | ||
this._videoTrack.duration = trackDuration; | ||
this.videoIntegrityChecker = | ||
this.config.handleMpegTsVideoIntegrityErrors === 'skip' | ||
? new PacketsIntegrityChecker(this.logger) | ||
: null; | ||
this._audioTrack = TSDemuxer.createTrack( | ||
'audio', | ||
trackDuration, | ||
|
@@ -228,6 +233,7 @@ class TSDemuxer implements Demuxer { | |
let pes: PES | null; | ||
|
||
const videoTrack = this._videoTrack as DemuxedVideoTrack; | ||
const videoIntegrityChecker = this.videoIntegrityChecker; | ||
const audioTrack = this._audioTrack as DemuxedAudioTrack; | ||
const id3Track = this._id3Track as DemuxedMetadataTrack; | ||
const textTrack = this._txtTrack as DemuxedUserdataTrack; | ||
|
@@ -291,15 +297,21 @@ class TSDemuxer implements Demuxer { | |
switch (pid) { | ||
case videoPid: | ||
if (stt) { | ||
if (videoData && (pes = parsePES(videoData, this.logger))) { | ||
if ( | ||
videoData && | ||
!videoIntegrityChecker?.isCorrupted && | ||
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. @mstyura, shouldn't this be repeated in 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. 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. 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. Please make the change if you don't mind. I noticed the issue while rebasing in github. Thanks for being patient with this one. 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. Sorry it took longer than I promised, but now it's rebased and updated. |
||
(pes = parsePES(videoData, this.logger)) | ||
) { | ||
this.readyVideoParser(videoTrack.segmentCodec); | ||
if (this.videoParser !== null) { | ||
this.videoParser.parsePES(videoTrack, textTrack, pes, false); | ||
} | ||
} | ||
|
||
videoData = { data: [], size: 0 }; | ||
videoIntegrityChecker?.reset(videoPid); | ||
} | ||
videoIntegrityChecker?.handlePacket(data.subarray(start)); | ||
if (videoData) { | ||
videoData.data.push(data.subarray(offset, start + PACKET_LENGTH)); | ||
videoData.size += start + PACKET_LENGTH - offset; | ||
|
@@ -464,9 +476,14 @@ class TSDemuxer implements Demuxer { | |
const videoData = videoTrack.pesData; | ||
const audioData = audioTrack.pesData; | ||
const id3Data = id3Track.pesData; | ||
const videoIntegrityChecker = this.videoIntegrityChecker; | ||
// try to parse last PES packets | ||
let pes: PES | null; | ||
if (videoData && (pes = parsePES(videoData, this.logger))) { | ||
if ( | ||
videoData && | ||
!videoIntegrityChecker?.isCorrupted && | ||
(pes = parsePES(videoData, this.logger)) | ||
) { | ||
this.readyVideoParser(videoTrack.segmentCodec); | ||
if (this.videoParser !== null) { | ||
this.videoParser.parsePES( | ||
|
@@ -590,6 +607,8 @@ class TSDemuxer implements Demuxer { | |
this._id3Track = | ||
this._txtTrack = | ||
undefined; | ||
|
||
this.videoIntegrityChecker = null; | ||
} | ||
|
||
private parseAACPES(track: DemuxedAudioTrack, pes: PES) { | ||
|
@@ -1050,4 +1069,77 @@ function parsePES(stream: ElementaryStreamData, logger: ILogger): PES | null { | |
return null; | ||
} | ||
|
||
// See FFMpeg for reference: https://github.com/FFmpeg/FFmpeg/blob/e4c8e80a2efee275f2a10fcf0424c9fc1d86e309/libavformat/mpegts.c#L2811-L2834 | ||
class PacketsIntegrityChecker { | ||
private readonly logger: ILogger; | ||
|
||
private pid: number = 0; | ||
private lastContinuityCounter = -1; | ||
private integrityState: 'ok' | 'tei-bit' | 'cc-failed' = 'ok'; | ||
|
||
constructor(logger: ILogger) { | ||
this.logger = logger; | ||
} | ||
|
||
public get isCorrupted(): boolean { | ||
return this.integrityState !== 'ok'; | ||
} | ||
|
||
public reset(pid: number) { | ||
this.pid = pid; | ||
this.lastContinuityCounter = -1; | ||
this.integrityState = 'ok'; | ||
} | ||
|
||
public handlePacket(data: Uint8Array) { | ||
if (data.byteLength < 4) { | ||
return; | ||
} | ||
|
||
const pid = parsePID(data, 0); | ||
if (pid !== this.pid) { | ||
this.logger.debug(`Packet PID mismatch, expected ${this.pid} got ${pid}`); | ||
return; | ||
} | ||
|
||
const adaptationFieldControl = (data[3] & 0x30) >> 4; | ||
if (adaptationFieldControl === 0) { | ||
return; | ||
} | ||
const continuityCounter = data[3] & 0xf; | ||
|
||
const lastContinuityCounter = this.lastContinuityCounter; | ||
this.lastContinuityCounter = continuityCounter; | ||
|
||
const hasPayload = (adaptationFieldControl & 0b01) != 0; | ||
const hasAdaptation = (adaptationFieldControl & 0b10) != 0; | ||
const isDiscontinuity = | ||
hasAdaptation && data[4] != 0 && (data[5] & 0x80) != 0; | ||
|
||
if (isDiscontinuity) { | ||
return; | ||
} | ||
if (lastContinuityCounter < 0) { | ||
return; | ||
} | ||
|
||
const expectedContinuityCounter = hasPayload | ||
? (lastContinuityCounter + 1) & 0x0f | ||
: lastContinuityCounter; | ||
if (continuityCounter !== expectedContinuityCounter) { | ||
this.logger.warn( | ||
`MPEG-TS Continuity Counter check failed for PID='${pid}', CC=${continuityCounter}, Expected-CC=${expectedContinuityCounter} Last-CC=${lastContinuityCounter}`, | ||
); | ||
this.integrityState = 'cc-failed'; | ||
return; | ||
} | ||
|
||
if ((data[1] & 0x80) !== 0) { | ||
this.logger.warn(`MPEG-TS Packet had TEI flag set for PID='${pid}'`); | ||
this.integrityState = 'tei-bit'; | ||
return; | ||
} | ||
} | ||
} | ||
|
||
export default TSDemuxer; |
Uh oh!
There was an error while loading. Please reload this page.
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.
@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.
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.
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.