GNOME Bugzilla – Bug 650416
[h264parse] Assertion failure: nal_size >=2
Last modified: 2011-05-23 14:00:20 UTC
Created attachment 187977 [details] I tried GST_DEBUG=5 but had to stop it when the log file hit 7 GB. I ran this pipeline, which until recently was failing due to #650323 ./gst-jhbuild run gst-launch-0.10 filesrc location=clip.mts ! mpegtsdemux name=demux ! ac3parse ! queue ! matroskamux name=mux ! filesink location=clip.mkv . demux. ! h264parse ! queue ! mux. The pipeline runs for a long time, as it should because clip.mts is a 3.5GB AVCHD file, and then fails with ** (gst-launch-0.10:10133): CRITICAL **: gst_h264_parse_process_nal: assertion `nal_size >= 2' failed Where I come from, an assertion failure is always a bug. Unfortunately, a 90 MB segment from the beginning of the file appears to be insufficient to trigger the failure, which does not occur until 2.9 GB have been written to clip.mkv.
Could you run this with G_DEBUG=fatal_warnings in gdb and get a backtrace of the warning? There are a few places in h264parse where gst_h264_parse_process_nal() might be called with nal_size < 2.
Created attachment 188025 [details] Backtrace with G_DEBUG=fatal_warnings Backtrace as requested. As you can see, it looks like the path is from parse_check_valid_frame().
Thanks, looks like not only the length is wrong but also the data.
Maybe the input data is basically wrong/corrupt. I seem to recall having put that assertion on at least size 2 mainly based on a valid NAL at least having size 2 (afaik). Whether or not the latter is actually correct (w.r.t. specs), it is likely not the best thing to be assert'ing on (and better some data sanity check/warning instead).
(In reply to comment #4) > it is likely not the best thing to be assert'ing on (and better some data > sanity check/warning instead). That sounds like a great idea. The input file here is straight off an AVCHD camera, so it's something that gstreamer should probably handle gracefully. It looks like the assertion happens right at the end of the file, so perhaps the camera truncated its output at a non-spec-compliant location. As it stands, the assertion prevents me from remuxing the file into a valid output, because the pipeline gets killed before matroskamux has a chance to finalize the .mkv file.
Created attachment 188084 [details] [review] Patch to fix the bug as suggested This patch fixes the bug, AFAICT. Rather than remove the assertion in parse_process_nal, it checks it upstream in check_valid_frame. This makes sense, I think, because it allows us to return FALSE from check_valid_frame if the frame is indeed invalid. It almost looks like the code was designed to have this check, given the comment above parse_process_nal that "caller guarantees 2 bytes of nal payload". Needs review.
Made some modifications bearing in mind that NALs may or may not be aggregated into AUs. Also, _process_nal is also called from a few other places, so better also make sure in _process_nal it is not being called with less than expected. commit fe2db9d46037b6daad2555051a40963e8d2f5e7d Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Mon May 23 14:41:27 2011 +0200 h264parse: gracefully handle truncated input NAL units Rather than assert'ing in such case, emit warning if the length of a NAL unit is less than expected 2 and discard it. Based on patch by Benjamin M. Schwartz <bens@alum.mit.edu> Fixes #650416.
Actually make that ... commit cce4fd073cd0b53fbf849b5a445a0a56ca3f2c67 Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Mon May 23 14:41:27 2011 +0200