Skip to content
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

ts-inspector crashes on keyframes without PTS/DTS #250

Closed
cmacq2 opened this issue Mar 13, 2019 · 2 comments
Closed

ts-inspector crashes on keyframes without PTS/DTS #250

cmacq2 opened this issue Mar 13, 2019 · 2 comments

Comments

@cmacq2
Copy link
Contributor

cmacq2 commented Mar 13, 2019

I came across a TypeError where a bit of ts-inspector code tried to assign a type property on a null object while debugging playback in video.js 7.4 on certain HLS streams that use TS fragments.

Tracing the error back to the source, it turns out that when the first key frame is encountered at line 223 of lib/tools/ts-inspector.js, an attempt is made to parse the PTS/DTS timestamps using the parsePesTime function from lib/m2ts/probe.js. Looking at this function, that can actually return null on a number of 'error' conditions, in particular when no PTS/DTS information is included with the keyframe.

After hacking up a Node JS script to run the affected segment(s) through ts-inspector.js to reproduce the crash and adding a bunch of console.log() calls, it turned out that, yes, these segments did in fact not appear to contain PTS/DTS info. I verified this by looking at the bytes I logged, assuming the offsets used by probe.js were correct: on my example segments both the PES flags and size fields were consistently zero.

For HLS playback in Chromium and Firefox, at least, this can be fixed by guarding against null in ts-inspector.js. Simply not setting firstKeyframe in the result object seems to work fine.

@mjneil
Copy link
Contributor

mjneil commented Mar 13, 2019

Can you share the problem segment? I'm curious how playback can even work without PTS/DTS information on the key frame. I saw your fix in #251 and while I don't really have an issue with adding an error check, I'm concerned that there is some other bug in our PES parsing that is incorrectly causing a null PTS/DTS

@cmacq2
Copy link
Contributor Author

cmacq2 commented Mar 13, 2019

Can you share the problem segment?

No, unfortunately I can't do that.

To give a bit more context about the segments:

ffprobe is also definitely not happy with any of the segments in general. With the segments which work even without this patch, it complains about PTS/DTS being basically out of whack. With the segments which are affected by this issue in mux.js, it doesn't even get to the stage where it complains about timestamp values which is probably because PTS/DTS would be missing entirely.

I am not sure if the following bit about how the segments are obtained is going to be all that helpful, since it seems that this problem only appears in segments which ultimately trace back to some specific sources but not others. With other sources, there is no lack of PTS/DTS info on the keyframes so the problem does not occur.

With that said: the segments are recorded from (multicast) UDP input with GStreamer 1.14 using roughly the following pipeline:

gst-launch-1.0 udpsrc ... ! tsdemux ! h264parse config-interval=1 ! hlssink2 ...

The original source is, in fact, RTSP but for $reasons it is received and resent as (multicast) UDP first. The sender is also a (rather older) GStreamer pipeline, similarly to this:

gst-launch-1.0 rtspsrc ... ! rtph264depay ! h264parse ! mpegtsmux alignment=7 ! udpsink ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants