From bfc4812bbec1b347a9e65c624c179e672b036728 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Tue, 26 Nov 2024 13:52:03 -0500 Subject: [PATCH] audiorate: convert next_ts to new segment instead of restarting from 0 When receiving a new segment we should not restart PTS from the new segment' start. Instead convert current position into the new segment if possible. Fixes: #4060 Part-of: --- .../gst/audiorate/gstaudiorate.c | 62 +++++++----- .../gst/audiorate/gstaudiorate.h | 1 - .../tests/check/elements/audiorate.c | 97 +++++++++++++++++++ 3 files changed, 136 insertions(+), 24 deletions(-) diff --git a/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.c b/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.c index 7b46037a91..8b6cbd3839 100644 --- a/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.c +++ b/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.c @@ -292,6 +292,24 @@ gst_audio_rate_fill_to_time (GstAudioRate * audiorate, GstClockTime time) gst_audio_rate_chain (audiorate->sinkpad, GST_OBJECT_CAST (audiorate), buf); } +/* FIXME: videorate has a copy, should it be public API? */ +static guint64 +convert_position (GstSegment * old_segment, GstSegment * new_segment, + guint64 position) +{ + g_return_val_if_fail (old_segment->format == new_segment->format, -1); + if (position == -1) + return -1; + position += old_segment->base; + if (position < new_segment->base) + return -1; + position -= new_segment->base; + if (position < new_segment->start || (new_segment->stop != -1 + && position > new_segment->stop)) + return -1; + return position; +} + static gboolean gst_audio_rate_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) { @@ -321,30 +339,28 @@ gst_audio_rate_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) case GST_EVENT_SEGMENT: { gst_event_copy_segment (event, &audiorate->sink_segment); - - GST_DEBUG_OBJECT (audiorate, "handle NEWSEGMENT"); -#if 0 - /* FIXME: bad things will likely happen if rate < 0 ... */ - if (!update) { - /* a new segment starts. We need to figure out what will be the next - * sample offset. We mark the offsets as invalid so that the _chain - * function will perform this calculation. */ - gst_audio_rate_fill_to_time (audiorate, audiorate->src_segment.stop); -#endif - audiorate->next_offset = -1; - audiorate->next_ts = -1; -#if 0 - } else { - gst_audio_rate_fill_to_time (audiorate, audiorate->src_segment.start); - } -#endif - GST_DEBUG_OBJECT (audiorate, "updated segment: %" GST_SEGMENT_FORMAT, &audiorate->sink_segment); + GstSegment old_segment; + gst_segment_copy_into (&audiorate->src_segment, &old_segment); + /* Copy sink_segment into src_segment and convert to TIME format. */ gst_audio_rate_convert_segments (audiorate); + /* Convert next_ts to new segment. */ + audiorate->next_ts = + convert_position (&old_segment, &audiorate->src_segment, + audiorate->next_ts); + if (audiorate->next_ts != -1) { + audiorate->next_offset = + gst_util_uint64_scale_int_round (audiorate->next_ts, + GST_AUDIO_INFO_RATE (&audiorate->info), GST_SECOND); + } else { + /* Current position is outside the new segment, _chain will resync. */ + audiorate->next_offset = -1; + } + /* Push updated segment */ guint32 seqnum = gst_event_get_seqnum (event); gst_event_take (&event, gst_event_new_segment (&audiorate->src_segment)); @@ -467,14 +483,14 @@ gst_audio_rate_chain (GstPad * pad, GstObject * parent, GstBuffer * buf) if (bpf == 0) goto not_negotiated; - /* we have a new pending segment */ if (audiorate->next_offset == -1) { gint64 pos; - /* first buffer, we are negotiated and we have a segment, calculate the - * current expected offsets based on the segment.start, which is the first - * media time of the segment and should match the media time of the first - * buffer in that segment, which is the offset expressed in DEFAULT units. + /* first buffer, or previous buffer's position was outside of new segment, + * calculate the current expected offsets based on the segment.start, which + * is the first media time of the segment and should match the media time of + * the first buffer in that segment, which is the offset expressed in + * DEFAULT units. */ /* convert first timestamp of segment to sample position */ pos = gst_util_uint64_scale_int_round (audiorate->src_segment.start, diff --git a/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.h b/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.h index 1c8fd16cbb..82ebf6bd11 100644 --- a/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.h +++ b/subprojects/gst-plugins-base/gst/audiorate/gstaudiorate.h @@ -54,7 +54,6 @@ struct _GstAudioRate gboolean discont; - gboolean new_segment; /* we accept all formats on the sink */ GstSegment sink_segment; /* we output TIME format on the src */ diff --git a/subprojects/gst-plugins-base/tests/check/elements/audiorate.c b/subprojects/gst-plugins-base/tests/check/elements/audiorate.c index 2363d71662..b3e7ff9604 100644 --- a/subprojects/gst-plugins-base/tests/check/elements/audiorate.c +++ b/subprojects/gst-plugins-base/tests/check/elements/audiorate.c @@ -564,6 +564,102 @@ GST_START_TEST (test_rate_change_down) GST_END_TEST; +static GstPadProbeReturn +segment_update_probe_cb (GstPad * pad, + GstPadProbeInfo * info, gpointer user_data) +{ + GstEvent *event = GST_PAD_PROBE_INFO_EVENT (info); + GList **events = user_data; + + *events = g_list_append (*events, gst_event_ref (event)); + return GST_PAD_PROBE_OK; +} + +GST_START_TEST (test_segment_update) +{ + GstElement *audiorate; + GstCaps *caps; + GstPad *srcpad, *sinkpad; + GstBuffer *buf; + + audiorate = gst_check_setup_element ("audiorate"); + caps = gst_caps_new_simple ("audio/x-raw", + "format", G_TYPE_STRING, GST_AUDIO_NE (F32), + "layout", G_TYPE_STRING, "interleaved", + "channels", G_TYPE_INT, 1, "rate", G_TYPE_INT, 44100, NULL); + srcpad = gst_check_setup_src_pad (audiorate, &srctemplate); + sinkpad = gst_check_setup_sink_pad (audiorate, &sinktemplate); + + gst_pad_set_active (srcpad, TRUE); + gst_check_setup_events (srcpad, audiorate, caps, GST_FORMAT_TIME); + gst_pad_set_active (sinkpad, TRUE); + fail_unless (gst_element_set_state (audiorate, + GST_STATE_PLAYING) == GST_STATE_CHANGE_SUCCESS, + "failed to set audiorate playing"); + + /* Initial segment is [0, -1], first buffer has PTS=0 */ + GstClockTime pts = 0; + gsize frame_size = sizeof (gfloat) * 1; + buf = gst_buffer_new_and_alloc (frame_size); + GST_BUFFER_TIMESTAMP (buf) = pts; + gst_pad_push (srcpad, buf); + fail_unless_equals_int (g_list_length (buffers), 1); + fail_unless_equals_int64 (GST_BUFFER_PTS (buffers->data), pts); + gst_check_drop_buffers (); + + GList *events = NULL; + gst_pad_add_probe (srcpad, + GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM, + (GstPadProbeCallback) segment_update_probe_cb, &events, NULL); + + /* Set segment base time to 2nd frame's PTS */ + GstSegment seg; + gst_segment_init (&seg, GST_FORMAT_TIME); + seg.base = GST_FRAMES_TO_CLOCK_TIME (1, 44100); + gst_pad_push_event (srcpad, gst_event_new_segment (&seg)); + fail_unless_equals_int (g_list_length (events), 1); + g_clear_list (&events, (GDestroyNotify) gst_event_unref); + + /* PTS=0 is correct because of the segment base time */ + pts = 0; + buf = gst_buffer_new_and_alloc (frame_size); + GST_BUFFER_TIMESTAMP (buf) = pts; + gst_pad_push (srcpad, buf); + fail_unless_equals_int (g_list_length (buffers), 1); + fail_unless_equals_int64 (GST_BUFFER_PTS (buffers->data), pts); + gst_check_drop_buffers (); + + /* Push [0, -1] segment again with base time back to 0 */ + gst_segment_init (&seg, GST_FORMAT_TIME); + gst_pad_push_event (srcpad, gst_event_new_segment (&seg)); + fail_unless_equals_int (g_list_length (events), 1); + g_clear_list (&events, (GDestroyNotify) gst_event_unref); + + /* PTS of 3rd frame because base time is back to 0. + * +1 because of rounding error. + * audiorate used to output a buffer with PTS back to segment.start instead of + * continuing from its current position. */ + pts = GST_FRAMES_TO_CLOCK_TIME (2, 44100) + 1; + buf = gst_buffer_new_and_alloc (frame_size); + GST_BUFFER_TIMESTAMP (buf) = pts; + gst_pad_push (srcpad, buf); + fail_unless_equals_int (g_list_length (buffers), 1); + fail_unless_equals_int64 (GST_BUFFER_PTS (buffers->data), pts); + gst_check_drop_buffers (); + + gst_element_set_state (audiorate, GST_STATE_NULL); + gst_caps_unref (caps); + + g_clear_list (&events, (GDestroyNotify) gst_event_unref); + gst_check_drop_buffers (); + gst_check_teardown_sink_pad (audiorate); + gst_check_teardown_src_pad (audiorate); + + gst_object_unref (audiorate); +} + +GST_END_TEST; + static Suite * audiorate_suite (void) { @@ -581,6 +677,7 @@ audiorate_suite (void) tcase_add_test (tc_chain, test_perfect_stream_drop45_inject25); tcase_add_test (tc_chain, test_large_discont); tcase_add_test (tc_chain, test_rate_change_down); + tcase_add_test (tc_chain, test_segment_update); return s; }