From 082a8fcd5ef17c785cd5359c77fdedfc905c4927 Mon Sep 17 00:00:00 2001 From: Guillaume Desmottes Date: Wed, 20 Nov 2024 14:16:23 +0100 Subject: [PATCH] h264parse: parse unregistered SEI without user data We get loads of warnings when parsing videos from users: gsth264parser.c:1115:gst_h264_parser_parse_user_data_unregistered: No more remaining payload data to store gsth264parse.c:646:gst_h264_parse_process_sei: failed to parse one or more SEI message Those are raised because of unregistered SEI without user data. The spec does not explicitly state that unregistered SEI needs to have data and I suppose the UUID by itself can carry valuable information. FFmpeg also parses and exposes such SEI so there is no reason for us no too as well. Part-of: --- girs/GstVideo-1.0.gir | 2 +- .../gst-libs/gst/codecparsers/gsth264parser.c | 6 ----- .../gst/videoparsers/gsth264parse.c | 3 --- .../tests/check/elements/h264parse.c | 25 +++++++++++++++++++ .../gst-libs/gst/video/video-sei.c | 3 +-- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/girs/GstVideo-1.0.gir b/girs/GstVideo-1.0.gir index dc70a707f7..0d36971888 100644 --- a/girs/GstVideo-1.0.gir +++ b/girs/GstVideo-1.0.gir @@ -16746,7 +16746,7 @@ parameters. User Data Unregistered UUID - + SEI User Data Unregistered buffer diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/codecparsers/gsth264parser.c b/subprojects/gst-plugins-bad/gst-libs/gst/codecparsers/gsth264parser.c index 82ba8185f3..b9bd6c3ee5 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/codecparsers/gsth264parser.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/codecparsers/gsth264parser.c @@ -1111,12 +1111,6 @@ gst_h264_parser_parse_user_data_unregistered (GstH264NalParser * nalparser, READ_UINT8 (nr, data[i], 8); } - if (payload_size < 1) { - GST_WARNING ("No more remaining payload data to store"); - g_clear_pointer (&data, g_free); - return GST_H264_PARSER_BROKEN_DATA; - } - urud->data = data; GST_MEMDUMP ("SEI user data unregistered", data, payload_size); return GST_H264_PARSER_OK; diff --git a/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c b/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c index 89e235ae12..0fb24b82cb 100644 --- a/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c +++ b/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c @@ -623,9 +623,6 @@ gst_h264_parse_process_sei_user_data_unregistered (GstH264Parse * h264parse, { GstByteReader br; - if (urud->data == NULL || urud->size < 1) - return; - gst_byte_reader_init (&br, urud->data, urud->size); gst_video_parse_user_data_unregistered ((GstElement *) h264parse, diff --git a/subprojects/gst-plugins-bad/tests/check/elements/h264parse.c b/subprojects/gst-plugins-bad/tests/check/elements/h264parse.c index 97cc063414..a5bffd9a09 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/h264parse.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/h264parse.c @@ -1488,6 +1488,16 @@ GST_START_TEST (test_parse_sei_userdefinedunregistered) 0x1f, 0x00, 0x05, 0xff, 0x21, 0x7e, 0xff, 0x29, 0xb5, 0xff, 0xdc, 0x13 }; + // Same SEI as above but without data + const guint8 no_data_sei[] = { + 0x00, 0x00, 0x00, 0x20, 0x06, 0x05, 0x10, 0x4d, + 0x49, 0x53, 0x50, 0x6d, 0x69, 0x63, 0x72, 0x6f, + 0x73, 0x65, 0x63, 0x74, 0x69, 0x6d, 0x65, + /* IDR frame (doesn't match caps) */ + 0x00, 0x00, 0x00, 0x14, 0x65, 0x88, 0x84, 0x00, + 0x10, 0xff, 0xfe, 0xf6, 0xf0, 0xfe, 0x05, 0x36, + 0x56, 0x04, 0x50, 0x96, 0x7b, 0x3f, 0x53, 0xe1 + }; h = gst_harness_new ("h264parse"); @@ -1511,6 +1521,21 @@ GST_START_TEST (test_parse_sei_userdefinedunregistered) gst_buffer_unref (buf); + // Now try parsing an unregistered SEI without data + buf = gst_buffer_new_and_alloc (misb_sei_size); + gst_buffer_fill (buf, 0, no_data_sei, sizeof (no_data_sei)); + fail_unless_equals_int (gst_harness_push (h, buf), GST_FLOW_OK); + + buf = gst_harness_pull (h); + meta = gst_buffer_get_video_sei_user_data_unregistered_meta (buf); + fail_unless (meta != NULL); + + fail_unless (memcmp (meta->uuid, H264_MISP_MICROSECTIME, 16) == 0); + fail_unless (meta->data == NULL); + fail_unless (meta->size == 0); + + gst_buffer_unref (buf); + gst_harness_teardown (h); } diff --git a/subprojects/gst-plugins-base/gst-libs/gst/video/video-sei.c b/subprojects/gst-plugins-base/gst-libs/gst/video/video-sei.c index 0b981c4076..908daf0c1e 100644 --- a/subprojects/gst-plugins-base/gst-libs/gst/video/video-sei.c +++ b/subprojects/gst-plugins-base/gst-libs/gst/video/video-sei.c @@ -156,7 +156,7 @@ gst_video_sei_user_data_unregistered_meta_get_info (void) * gst_buffer_add_video_sei_user_data_unregistered_meta: * @buffer: a #GstBuffer * @uuid: User Data Unregistered UUID - * @data: (transfer none): SEI User Data Unregistered buffer + * @data: (transfer none) (allow-none): SEI User Data Unregistered buffer * @size: size of the data buffer * * Attaches #GstVideoSEIUserDataUnregisteredMeta metadata to @buffer with the given @@ -172,7 +172,6 @@ gst_buffer_add_video_sei_user_data_unregistered_meta (GstBuffer * buffer, { GstVideoSEIUserDataUnregisteredMeta *meta; g_return_val_if_fail (GST_IS_BUFFER (buffer), NULL); - g_return_val_if_fail (data != NULL, NULL); meta = (GstVideoSEIUserDataUnregisteredMeta *) gst_buffer_add_meta (buffer, GST_VIDEO_SEI_USER_DATA_UNREGISTERED_META_INFO, NULL);