From 6ddd51c23956dbcf00dd0849f15a3ac422383d60 Mon Sep 17 00:00:00 2001 From: "Jan Alexander Steffens (heftig)" Date: Tue, 6 Aug 2024 18:09:58 +0200 Subject: [PATCH] h264parse: Fix pic_timing SEI replacement The calculated position was off. I'm not sure of the exact cause; possibly because we're in AU-aligned byte-stream mode, which means `transform` is true. Replacing the math that calculates the NALU positions with code more similar to what is already in use for `idr_pos` seems to have fixed it. Part-of: --- .../gst/videoparsers/gsth264parse.c | 106 +++++++++++++++--- .../gst/videoparsers/gsth264parse.h | 2 +- 2 files changed, 90 insertions(+), 18 deletions(-) diff --git a/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c b/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c index b6be4a5f34..89e235ae12 100644 --- a/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c +++ b/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.c @@ -235,7 +235,7 @@ gst_h264_parse_reset_frame (GstH264Parse * h264parse) h264parse->idr_pos = -1; h264parse->sei_pos = -1; h264parse->pic_timing_sei_pos = -1; - h264parse->pic_timing_sei_size = -1; + h264parse->pic_timing_sei_end = -1; h264parse->keyframe = FALSE; h264parse->predicted = FALSE; h264parse->bidirectional = FALSE; @@ -677,9 +677,17 @@ gst_h264_parse_process_sei (GstH264Parse * h264parse, GstH264NalUnit * nalu) * Updating only this SEI message and preserving the others * is a bit complicated */ if (messages->len == 1) { - h264parse->pic_timing_sei_pos = nalu->sc_offset; - h264parse->pic_timing_sei_size = - nalu->size + (nalu->offset - nalu->sc_offset); + if (h264parse->transform) + h264parse->pic_timing_sei_pos = + gst_adapter_available (h264parse->frame_out); + else + h264parse->pic_timing_sei_pos = nalu->sc_offset; + + GST_LOG_OBJECT (h264parse, "Located pic_timing SEI at pos %i", + h264parse->pic_timing_sei_pos); + } else { + GST_FIXME_OBJECT (h264parse, + "Cannot update timecode in multi-message SEI NALU"); } } @@ -979,6 +987,19 @@ gst_h264_parse_process_nal (GstH264Parse * h264parse, GstH264NalUnit * nalu) GST_DEBUG_OBJECT (h264parse, "processing nal of type %u %s, size %u", nal_type, _nal_name (nal_type), nalu->size); + if (h264parse->pic_timing_sei_pos != -1 && + h264parse->pic_timing_sei_end == -1) { + if (h264parse->transform) + h264parse->pic_timing_sei_end = + gst_adapter_available (h264parse->frame_out); + else + h264parse->pic_timing_sei_end = nalu->sc_offset; + + GST_LOG_OBJECT (h264parse, "Located pic_timing SEI end at pos %i (size %i)", + h264parse->pic_timing_sei_end, + h264parse->pic_timing_sei_end - h264parse->pic_timing_sei_pos); + } + switch (nal_type) { case GST_H264_NAL_SUBSET_SPS: if (!GST_H264_PARSE_STATE_VALID (h264parse, GST_H264_PARSE_STATE_GOT_SPS)) @@ -3048,6 +3069,7 @@ gst_h264_parse_create_pic_timing_sei (GstH264Parse * h264parse, GstBuffer * buffer) { guint num_meta; + gint sei_pos, sei_end; const guint8 num_clock_ts_table[9] = { 1, 1, 1, 2, 2, 3, 3, 2, 3 }; @@ -3069,12 +3091,20 @@ gst_h264_parse_create_pic_timing_sei (GstH264Parse * h264parse, if (num_meta == 0) return NULL; - if (!h264parse->sei_pic_struct_pres_flag || h264parse->pic_timing_sei_pos < 0) { + sei_pos = h264parse->pic_timing_sei_pos; + sei_end = h264parse->pic_timing_sei_end; + + if (!h264parse->sei_pic_struct_pres_flag || sei_pos < 0) { GST_ELEMENT_WARNING (h264parse, STREAM, NOT_IMPLEMENTED, (NULL), ("timecode update was requested but VUI doesn't support timecode")); return NULL; } + if (h264parse->align != GST_H264_PARSE_ALIGN_NAL && sei_end < 0) { + GST_ELEMENT_WARNING (h264parse, STREAM, NOT_IMPLEMENTED, (NULL), + ("timecode update was requested but end of pic_timing SEI not found")); + } + g_assert (h264parse->sei_pic_struct <= GST_H264_SEI_PIC_STRUCT_FRAME_TRIPLING); @@ -3184,31 +3214,67 @@ gst_h264_parse_create_pic_timing_sei (GstH264Parse * h264parse, gst_buffer_copy_into (out_buf, buffer, GST_BUFFER_COPY_METADATA, 0, -1); if (h264parse->align == GST_H264_PARSE_ALIGN_NAL) { + GstMapInfo map; + + /* FIXME: We're replacing the entire buffer. + * Is this buffer always the pic_timing SEI? */ + + GST_DEBUG_OBJECT (h264parse, "Replacing buffer with new pic_timing SEI"); + + if (GST_LEVEL_MEMDUMP <= _gst_debug_min && + gst_buffer_map (buffer, &map, GST_MAP_READ)) { + GST_MEMDUMP_OBJECT (h264parse, "Old SEI", map.data, map.size); + gst_buffer_unmap (buffer, &map); + } + + if (GST_LEVEL_MEMDUMP <= _gst_debug_min && + gst_memory_map (sei_mem, &map, GST_MAP_READ)) { + GST_MEMDUMP_OBJECT (h264parse, "New SEI", map.data, map.size); + gst_memory_unmap (sei_mem, &map); + } + gst_buffer_append_memory (out_buf, sei_mem); } else { - gsize mem_size; + GstMapInfo map; + gint sei_size = sei_end - sei_pos; - mem_size = gst_memory_get_sizes (sei_mem, NULL, NULL); + GST_DEBUG_OBJECT (h264parse, "Replacing pic_timing SEI in buffer at %i " + "size %i with new size %i", sei_pos, sei_size, (gint) sei_mem->size); + + if (GST_LEVEL_MEMDUMP <= _gst_debug_min && sei_pos >= 0 && + gst_buffer_map (buffer, &map, GST_MAP_READ)) { + gint context_start = MIN (sei_pos, 16); + gint context_end = MIN (map.size - sei_end, 16); + + GST_MEMDUMP_OBJECT (h264parse, "Old SEI with up to 16 bytes context", + map.data + sei_pos - context_start, + context_start + sei_size + context_end); + GST_MEMDUMP_OBJECT (h264parse, "Old SEI", map.data + sei_pos, sei_size); + gst_buffer_unmap (buffer, &map); + } + + if (GST_LEVEL_MEMDUMP <= _gst_debug_min && + gst_memory_map (sei_mem, &map, GST_MAP_READ)) { + GST_MEMDUMP_OBJECT (h264parse, "New SEI", map.data, map.size); + gst_memory_unmap (sei_mem, &map); + } /* copy every data except for the SEI */ - if (h264parse->pic_timing_sei_pos > 0) { + if (sei_pos > 0) { gst_buffer_copy_into (out_buf, buffer, GST_BUFFER_COPY_MEMORY, 0, - h264parse->pic_timing_sei_pos); + sei_pos); } /* insert new SEI */ gst_buffer_append_memory (out_buf, sei_mem); - if (gst_buffer_get_size (buffer) > - h264parse->pic_timing_sei_pos + h264parse->pic_timing_sei_size) { - gst_buffer_copy_into (out_buf, buffer, GST_BUFFER_COPY_MEMORY, - h264parse->pic_timing_sei_pos + h264parse->pic_timing_sei_size, -1); + if (gst_buffer_get_size (buffer) > sei_end) { + gst_buffer_copy_into (out_buf, buffer, GST_BUFFER_COPY_MEMORY, sei_end, + -1); } - if (h264parse->idr_pos >= 0) { - h264parse->idr_pos += mem_size; - h264parse->idr_pos -= h264parse->pic_timing_sei_size; - } + if (h264parse->idr_pos >= 0) + h264parse->idr_pos += sei_mem->size - sei_size; } return out_buf; @@ -3269,6 +3335,12 @@ gst_h264_parse_pre_push_frame (GstBaseParse * parse, GstBaseParseFrame * frame) gst_buffer_prepend_memory (frame->out_buffer, mem); if (h264parse->idr_pos >= 0) h264parse->idr_pos += sizeof (au_delim); + if (h264parse->sei_pos >= 0) + h264parse->sei_pos += sizeof (au_delim); + if (h264parse->pic_timing_sei_pos >= 0) + h264parse->pic_timing_sei_pos += sizeof (au_delim); + if (h264parse->pic_timing_sei_end >= 0) + h264parse->pic_timing_sei_end += sizeof (au_delim); buffer = frame->out_buffer; } else { diff --git a/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.h b/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.h index 1b6939de31..caa87d384a 100644 --- a/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.h +++ b/subprojects/gst-plugins-bad/gst/videoparsers/gsth264parse.h @@ -134,7 +134,7 @@ struct _GstH264Parse /*guint next_sc_pos;*/ gint idr_pos, sei_pos; gint pic_timing_sei_pos; - gint pic_timing_sei_size; + gint pic_timing_sei_end; gboolean update_caps; GstAdapter *frame_out; gboolean keyframe;