h264parse: Properly handle 4 bytes start code

This will stop stripping four bytes start code. This was fixed and broken
again as it was causing the a timestamp shift. We now call
gst_base_parse_set_ts_at_offset() with the offset of the first NAL to ensure
that fixing a moderatly broken input stream won't affect the timestamps. We
also fixes the unit test, removing a comment about the stripping behaviour not
being correct.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1251>
This commit is contained in:
Nicolas Dufresne 2020-05-06 22:28:34 -04:00
parent 9029631f75
commit dc4c470d75
3 changed files with 21 additions and 48 deletions

View file

@ -1452,6 +1452,9 @@ gst_h264_parser_identify_nalu_unchecked (GstH264NalParser * nalparser,
nalu->sc_offset = offset + off1;
/* sc might have 2 or 3 0-bytes */
if (nalu->sc_offset > 0 && data[nalu->sc_offset - 1] == 00)
nalu->sc_offset--;
nalu->offset = offset + off1 + 3;
nalu->data = (guint8 *) data;
@ -1465,12 +1468,6 @@ gst_h264_parser_identify_nalu_unchecked (GstH264NalParser * nalparser,
nalu->valid = TRUE;
/* sc might have 2 or 3 0-bytes */
if (nalu->sc_offset > 0 && data[nalu->sc_offset - 1] == 00
&& (nalu->type == GST_H264_NAL_SPS || nalu->type == GST_H264_NAL_PPS
|| nalu->type == GST_H264_NAL_AU_DELIMITER))
nalu->sc_offset--;
if (nalu->type == GST_H264_NAL_SEQ_END ||
nalu->type == GST_H264_NAL_STREAM_END) {
GST_DEBUG ("end-of-seq or end-of-stream nal found");

View file

@ -1315,7 +1315,6 @@ gst_h264_parse_handle_frame (GstBaseParse * parse,
GstH264NalUnit nalu;
GstH264ParserResult pres;
gint framesize;
GstFlowReturn ret;
if (G_UNLIKELY (GST_BUFFER_FLAG_IS_SET (frame->buffer,
GST_BUFFER_FLAG_DISCONT))) {
@ -1380,22 +1379,6 @@ gst_h264_parse_handle_frame (GstBaseParse * parse,
switch (pres) {
case GST_H264_PARSER_OK:
if (nalu.sc_offset > 0) {
int i;
gboolean is_filler_data = TRUE;
/* Handle filler data */
for (i = 0; i < nalu.sc_offset; i++) {
if (data[i] != 0x00) {
is_filler_data = FALSE;
break;
}
}
if (is_filler_data) {
GST_DEBUG_OBJECT (parse, "Dropping filler data %d", nalu.sc_offset);
frame->flags |= GST_BASE_PARSE_FRAME_FLAG_DROP;
gst_buffer_unmap (buffer, &map);
ret = gst_base_parse_finish_frame (parse, frame, nalu.sc_offset);
goto drop;
}
*skipsize = nalu.sc_offset;
goto skip;
}
@ -1410,6 +1393,10 @@ gst_h264_parse_handle_frame (GstBaseParse * parse,
("Error parsing H.264 stream"), ("Invalid H.264 stream"));
goto invalid_stream;
}
/* Ensure we use the TS of the first NAL. This avoids broken timestamp in
* the case of a miss-placed filler byte. */
gst_base_parse_set_ts_at_offset (parse, nalu.offset);
}
while (TRUE) {
@ -1576,10 +1563,6 @@ out:
gst_buffer_unmap (buffer, &map);
return GST_FLOW_OK;
drop:
GST_DEBUG_OBJECT (h264parse, "Dropped data");
return ret;
skip:
GST_DEBUG_OBJECT (h264parse, "skipping %d", *skipsize);
/* If we are collecting access units, we need to preserve the initial

View file

@ -180,11 +180,9 @@ verify_buffer (buffer_verify_data_s * vdata, GstBuffer * buffer)
if (vdata->discard) {
/* check separate header NALs */
gint i = vdata->buffer_counter;
guint ofs;
gboolean aud;
/* SEI with start code prefix with 2 0-bytes */
ofs = i == 2;
aud = i == 0;
fail_unless (i <= 3);
@ -196,8 +194,8 @@ verify_buffer (buffer_verify_data_s * vdata, GstBuffer * buffer)
} else {
i -= 1;
fail_unless (gst_buffer_get_size (buffer) == ctx_headers[i].size - ofs);
fail_unless (gst_buffer_memcmp (buffer, 0, ctx_headers[i].data + ofs,
fail_unless (gst_buffer_get_size (buffer) == ctx_headers[i].size);
fail_unless (gst_buffer_memcmp (buffer, 0, ctx_headers[i].data,
gst_buffer_get_size (buffer)) == 0);
}
} else {
@ -895,11 +893,6 @@ composite_buffer (GstClockTime pts, GstBufferFlags flags, gint count, ...)
#define pull_and_check(h, data, pts, flags) \
pull_and_check_full (h, data, sizeof (data), pts, flags)
/* used to check NALs for which the parser removes the first 0x00 byte;
* this parser behavior is a bit broken, so we may remove that in the future */
#define pull_and_check_skip1byte(h, data, pts, flags) \
pull_and_check_full (h, data + 1, sizeof (data) - 1, pts, flags)
#define pull_and_drop(h) \
G_STMT_START { \
GstBuffer *b = gst_harness_pull (h); \
@ -936,29 +929,29 @@ GST_START_TEST (test_parse_sliced_nal_nal)
buf = wrap_buffer (h264_idr_slice_1, sizeof (h264_idr_slice_1), 100, 0);
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 1);
pull_and_check_skip1byte (h, h264_idr_slice_1, 100, 0);
pull_and_check (h, h264_idr_slice_1, 100, 0);
buf = wrap_buffer (h264_idr_slice_2, sizeof (h264_idr_slice_2), 100, 0);
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 1);
pull_and_check_skip1byte (h, h264_idr_slice_2, -1, 0);
pull_and_check (h, h264_idr_slice_2, -1, 0);
buf = wrap_buffer (h264_idr_slice_1, sizeof (h264_idr_slice_1), 200, 0);
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 2);
pull_and_check (h, h264_aud, 200, 0);
pull_and_check_skip1byte (h, h264_idr_slice_1, 200, 0);
pull_and_check (h, h264_idr_slice_1, 200, 0);
buf = wrap_buffer (h264_idr_slice_2, sizeof (h264_idr_slice_2), 200, 0);
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 1);
pull_and_check_skip1byte (h, h264_idr_slice_2, -1, 0);
pull_and_check (h, h264_idr_slice_2, -1, 0);
buf = wrap_buffer (h264_idr_slice_1, sizeof (h264_idr_slice_1), 250, 0);
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 2);
pull_and_check (h, h264_aud, 250, 0);
pull_and_check_skip1byte (h, h264_idr_slice_1, 250, 0);
pull_and_check (h, h264_idr_slice_1, 250, 0);
/* 1st slice starts a new AU, even though the previous one is incomplete.
* DISCONT must also be propagated */
@ -967,7 +960,7 @@ GST_START_TEST (test_parse_sliced_nal_nal)
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 2);
pull_and_check (h, h264_aud, 400, 0);
pull_and_check_skip1byte (h, h264_idr_slice_1, 400, GST_BUFFER_FLAG_DISCONT);
pull_and_check (h, h264_idr_slice_1, 400, GST_BUFFER_FLAG_DISCONT);
gst_harness_teardown (h);
}
@ -1004,8 +997,8 @@ GST_START_TEST (test_parse_sliced_au_nal)
/* 1st slice here doens't have a PTS
* because it was present in the first header NAL */
pull_and_check_skip1byte (h, h264_idr_slice_1, -1, 0);
pull_and_check_skip1byte (h, h264_idr_slice_2, -1, 0);
pull_and_check (h, h264_idr_slice_1, -1, 0);
pull_and_check (h, h264_idr_slice_2, -1, 0);
/* new AU. we expect AUD to be inserted and 1st slice to have the same PTS */
buf = composite_buffer (200, 0, 2,
@ -1014,8 +1007,8 @@ GST_START_TEST (test_parse_sliced_au_nal)
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 3);
pull_and_check (h, h264_aud, 200, 0);
pull_and_check_skip1byte (h, h264_idr_slice_1, 200, 0);
pull_and_check_skip1byte (h, h264_idr_slice_2, -1, 0);
pull_and_check (h, h264_idr_slice_1, 200, 0);
pull_and_check (h, h264_idr_slice_2, -1, 0);
/* DISCONT must be propagated */
buf = composite_buffer (400, GST_BUFFER_FLAG_DISCONT, 2,
@ -1024,8 +1017,8 @@ GST_START_TEST (test_parse_sliced_au_nal)
fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK);
fail_unless_equals_int (gst_harness_buffers_in_queue (h), 3);
pull_and_check (h, h264_aud, 400, 0);
pull_and_check_skip1byte (h, h264_idr_slice_1, 400, GST_BUFFER_FLAG_DISCONT);
pull_and_check_skip1byte (h, h264_idr_slice_2, -1, 0);
pull_and_check (h, h264_idr_slice_1, 400, GST_BUFFER_FLAG_DISCONT);
pull_and_check (h, h264_idr_slice_2, -1, 0);
gst_harness_teardown (h);
}