From 92a4cfe20f8c62146cbc0726214ac1456d06d365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Oca=C3=B1a=20Gonz=C3=A1lez?= Date: Wed, 1 Feb 2023 12:09:52 +0100 Subject: [PATCH] qtdemux: Don't emit GstSegment correcting start time when in MSE mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using qtdemux in a pipeline that should only work as a pure demuxer (not for actual playback), qtdemux shouldn't emit new GstSegments to correct the start time (jump to the future) to ensure that the user experiences no playback delay. By doing so, it's generating the wrong segments when an append of data from the past happens. When that happens, downstream elements such as parsers (eg: aacparse) may clip those buffers laying before the GstSegment and create problems on the GStreamer client app (eg: WebKit). Getting buffers clipped out because of the wrong GstSegments started becoming a problen when this commit was introduced: ab6e49e9cc audioparsers: add back segment clipping to parsers that have lost it This clipping makes test DASH shaka 35 from MVT tests[1] to fail in WebKitGTK/WPE (at least) and can potentially cause a number of other problems in the WebKit Media Source Extensions (MSE) code. Note that this new behaviour of not emitting new GstSegments only makes sense when qtdemux is being used as a pure demuxer and not as part of a regular pipeline. This is why the variant field has been added. When equal to VARIANT_MSE_BYTESTREAM, it will make qtdemux behave differently in push mode, taking decisions that meet the expectations for an MSE-like processing mode. This kind of tweaks have been done in the past for MSS streams, for instance. That code has been refactored to use VARIANT_MSS_FRAGMENTED now, instead of its own dedicated boolean flag. Co-authored by: Alicia Boya García ...who suggested to use "variant: mse-bytestream" in the caps to identify that mode, as proposed in her unmerged patch: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/issues/467 [1] https://github.com/rdkcentral/mvt Part-of: --- subprojects/gst-integration-testsuites/medias | 2 +- .../testsuites/validate.testslist | 3 +++ .../segment_future_past_mse.validatetest | 22 +++++++++++++++++++ .../log-fakesink0-sink-expected | 17 ++++++++++++++ ...nt_future_past_mse_edit_lists.validatetest | 22 +++++++++++++++++++ .../log-fakesink0-sink-expected | 17 ++++++++++++++ .../segment_future_past_nomse.validatetest | 22 +++++++++++++++++++ .../log-fakesink0-sink-expected | 18 +++++++++++++++ .../gst-plugins-good/gst/isomp4/qtdemux.c | 22 +++++++++++-------- .../gst-plugins-good/gst/isomp4/qtdemux.h | 18 +++++++++++---- 10 files changed, 149 insertions(+), 14 deletions(-) create mode 100644 subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse.validatetest create mode 100644 subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse/flow-expectations/log-fakesink0-sink-expected create mode 100644 subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists.validatetest create mode 100644 subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists/flow-expectations/log-fakesink0-sink-expected create mode 100644 subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse.validatetest create mode 100644 subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse/flow-expectations/log-fakesink0-sink-expected diff --git a/subprojects/gst-integration-testsuites/medias b/subprojects/gst-integration-testsuites/medias index bc766cb9a2..2ec828b7e7 160000 --- a/subprojects/gst-integration-testsuites/medias +++ b/subprojects/gst-integration-testsuites/medias @@ -1 +1 @@ -Subproject commit bc766cb9a23c38bfc68dda05349a40edd5d06937 +Subproject commit 2ec828b7e76cb3240bcc546990e6b3dedceecb40 diff --git a/subprojects/gst-integration-testsuites/testsuites/validate.testslist b/subprojects/gst-integration-testsuites/testsuites/validate.testslist index cc3db2ac6c..3a87d99af1 100644 --- a/subprojects/gst-integration-testsuites/testsuites/validate.testslist +++ b/subprojects/gst-integration-testsuites/testsuites/validate.testslist @@ -942,6 +942,9 @@ validate.test.matroska.demux_flush_within_cluster.default validate.test.mp4.qtdemux_change_edit_list.default validate.test.mp4.qtdemux_reverse_playback_full_gop.reverse_playback_full_gop validate.test.mp4.redirect.play_15s +validate.test.mse.segment_future_past_mse.segment_future_past_mse +validate.test.mse.segment_future_past_mse_edit_lists.segment_future_past_mse_edit_lists +validate.test.mse.segment_future_past_nomse.segment_future_past_nomse validate.test.nle.urisource.play validate.test.playbin.check_active_stream validate.test.playbin3.ignore_raw_audio_from_demuxer diff --git a/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse.validatetest b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse.validatetest new file mode 100644 index 0000000000..396b5f62be --- /dev/null +++ b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse.validatetest @@ -0,0 +1,22 @@ +set-globals, media_dir="$(test_dir)/../../../../medias/fragments/mse.1/" +meta, + seek=false, + handles-states=false, + args = { + "appsrc ! video/quicktime, variant=mse-bytestream ! qtdemux ! fakesink async=false", + }, + configs = { + "$(validateflow), pad=fakesink0:sink, record-buffers=false, logged-event-types={segment}", + } + +# Scenario action types +checkpoint, text="Mse-bytestream variant" +checkpoint, text="Init segment is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-init.mp4" +checkpoint, text="Data from the future is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-media-future.mp4" +checkpoint, text="Init segment is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-init.mp4" +checkpoint, text="Data from the past is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-media-past.mp4" +stop diff --git a/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse/flow-expectations/log-fakesink0-sink-expected b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse/flow-expectations/log-fakesink0-sink-expected new file mode 100644 index 0000000000..10e570bc6d --- /dev/null +++ b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse/segment_future_past_mse/flow-expectations/log-fakesink0-sink-expected @@ -0,0 +1,17 @@ + +CHECKPOINT: Mse-bytestream variant + + +CHECKPOINT: Init segment is now pushed + +event segment: format=TIME, start=0:00:00.000000000, offset=0:00:00.000000000, stop=none, time=0:00:00.000000000, base=0:00:00.000000000, position=0:00:00.000000000 + +CHECKPOINT: Data from the future is now pushed + + +CHECKPOINT: Init segment is now pushed + +event segment: format=TIME, start=0:00:00.000000000, offset=0:00:00.000000000, stop=none, time=0:00:00.000000000, base=0:00:00.000000000, position=0:00:00.000000000 + +CHECKPOINT: Data from the past is now pushed + diff --git a/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists.validatetest b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists.validatetest new file mode 100644 index 0000000000..8d6c616450 --- /dev/null +++ b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists.validatetest @@ -0,0 +1,22 @@ +set-globals, media_dir="$(test_dir)/../../../../medias/fragments/" +meta, + seek=false, + handles-states=false, + args = { + "appsrc ! video/quicktime, variant=mse-bytestream ! qtdemux ! fakesink async=false", + }, + configs = { + "$(validateflow), pad=fakesink0:sink, record-buffers=false, logged-event-types={segment}", + } + +# Scenario action types +checkpoint, text="Mse-bytestream variant" +checkpoint, text="Init segment is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/car-20120827-86.mp4/init.mp4" +checkpoint, text="Data from the future is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/car-20120827-86.mp4/media2.mp4" +checkpoint, text="Init segment is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/car-20120827-86.mp4/init.mp4" +checkpoint, text="Data from the past is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/car-20120827-86.mp4/media1.mp4" +stop diff --git a/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists/flow-expectations/log-fakesink0-sink-expected b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists/flow-expectations/log-fakesink0-sink-expected new file mode 100644 index 0000000000..83233325b8 --- /dev/null +++ b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_mse_edit_lists/segment_future_past_mse_edit_lists/flow-expectations/log-fakesink0-sink-expected @@ -0,0 +1,17 @@ + +CHECKPOINT: Mse-bytestream variant + + +CHECKPOINT: Init segment is now pushed + +event segment: format=TIME, start=0:00:00.041711111, offset=0:00:00.000000000, stop=none, time=0:00:00.000000000, base=0:00:00.000000000, position=0:00:00.041711111 + +CHECKPOINT: Data from the future is now pushed + + +CHECKPOINT: Init segment is now pushed + +event segment: format=TIME, start=0:00:00.041711111, offset=0:00:00.000000000, stop=none, time=0:00:00.000000000, base=0:00:00.000000000, position=0:00:00.041711111 + +CHECKPOINT: Data from the past is now pushed + diff --git a/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse.validatetest b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse.validatetest new file mode 100644 index 0000000000..41a7c3e5e1 --- /dev/null +++ b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse.validatetest @@ -0,0 +1,22 @@ +set-globals, media_dir="$(test_dir)/../../../../medias/fragments/mse.1/" +meta, + seek=false, + handles-states=false, + args = { + "appsrc ! qtdemux ! fakesink async=false", + }, + configs = { + "$(validateflow), pad=fakesink0:sink, record-buffers=false, logged-event-types={segment}", + } + +# Scenario action types +checkpoint, text="No mse-bytestream variant" +checkpoint, text="Init segment is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-init.mp4" +checkpoint, text="Data from the future is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-media-future.mp4" +checkpoint, text="Init segment is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-init.mp4" +checkpoint, text="Data from the past is now pushed" +appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/segment-mse-first-sample-media-past.mp4" +stop diff --git a/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse/flow-expectations/log-fakesink0-sink-expected b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse/flow-expectations/log-fakesink0-sink-expected new file mode 100644 index 0000000000..59208490bd --- /dev/null +++ b/subprojects/gst-integration-testsuites/testsuites/validate/mse/segment_future_past_nomse/segment_future_past_nomse/flow-expectations/log-fakesink0-sink-expected @@ -0,0 +1,18 @@ + +CHECKPOINT: No mse-bytestream variant + + +CHECKPOINT: Init segment is now pushed + +event segment: format=TIME, start=0:00:00.000000000, offset=0:00:00.000000000, stop=none, time=0:00:00.000000000, base=0:00:00.000000000, position=0:00:00.000000000 + +CHECKPOINT: Data from the future is now pushed + +event segment: format=TIME, start=465162:45:48.010666752, offset=0:00:00.000000000, stop=none, time=465162:45:48.010666666, base=0:00:00.000000000, position=465162:45:48.010666752 + +CHECKPOINT: Init segment is now pushed + +event segment: format=TIME, start=465162:45:48.010666752, offset=0:00:00.000000000, stop=none, time=465162:45:48.010666666, base=0:00:00.000000000, position=465162:45:48.010666752 + +CHECKPOINT: Data from the past is now pushed + diff --git a/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c b/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c index f5894303a3..234c273ce5 100644 --- a/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c +++ b/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c @@ -1893,12 +1893,16 @@ gst_qtdemux_setcaps (GstQTDemux * demux, GstCaps * caps) structure = gst_caps_get_structure (caps, 0); variant = gst_structure_get_string (structure, "variant"); + if (variant && strcmp (variant, "mse-bytestream") == 0) { + demux->variant = VARIANT_MSE_BYTESTREAM; + } + if (variant && strcmp (variant, "mss-fragmented") == 0) { QtDemuxStream *stream; const GValue *value; demux->fragmented = TRUE; - demux->mss_mode = TRUE; + demux->variant = VARIANT_MSS_FRAGMENTED; if (QTDEMUX_N_STREAMS (demux) > 1) { /* can't do this, we can only renegotiate for another mss format */ @@ -1969,8 +1973,6 @@ gst_qtdemux_setcaps (GstQTDemux * demux, GstCaps * caps) } } gst_caps_replace (&demux->media_caps, (GstCaps *) mediacaps); - } else { - demux->mss_mode = FALSE; } return TRUE; @@ -2058,7 +2060,7 @@ gst_qtdemux_reset (GstQTDemux * qtdemux, gboolean hard) qtdemux->n_meta_streams = 0; qtdemux->exposed = FALSE; qtdemux->fragmented = FALSE; - qtdemux->mss_mode = FALSE; + qtdemux->variant = VARIANT_NONE; gst_caps_replace (&qtdemux->media_caps, NULL); qtdemux->timescale = 0; qtdemux->got_moov = FALSE; @@ -2079,7 +2081,7 @@ gst_qtdemux_reset (GstQTDemux * qtdemux, gboolean hard) g_free (qtdemux->preferred_protection_system_id); qtdemux->preferred_protection_system_id = NULL; } - } else if (qtdemux->mss_mode) { + } else if (qtdemux->variant == VARIANT_MSS_FRAGMENTED) { gst_flow_combiner_reset (qtdemux->flowcombiner); g_ptr_array_foreach (qtdemux->active_streams, (GFunc) gst_qtdemux_stream_clear, NULL); @@ -3625,7 +3627,7 @@ qtdemux_find_stream (GstQTDemux * qtdemux, guint32 id) if (stream->track_id == id) return stream; } - if (qtdemux->mss_mode) { + if (qtdemux->variant == VARIANT_MSS_FRAGMENTED) { /* mss should have only 1 stream anyway */ return QTDEMUX_NTH_STREAM (qtdemux, 0); } @@ -3692,7 +3694,7 @@ qtdemux_parse_tfhd (GstQTDemux * qtdemux, GstByteReader * tfhd, (*stream)->stsd_sample_description_id = sample_description_index - 1; } - if (qtdemux->mss_mode) { + if (qtdemux->variant == VARIANT_MSS_FRAGMENTED) { /* mss has no stsd entry */ (*stream)->stsd_sample_description_id = 0; } @@ -4295,7 +4297,9 @@ qtdemux_parse_moof (GstQTDemux * qtdemux, const guint8 * buffer, guint length, pssh_node = qtdemux_tree_get_sibling_by_type (pssh_node, FOURCC_pssh); } - if (!qtdemux->upstream_format_is_time && !qtdemux->first_moof_already_parsed + if (!qtdemux->upstream_format_is_time + && qtdemux->variant != VARIANT_MSE_BYTESTREAM + && !qtdemux->first_moof_already_parsed && !qtdemux->received_seek && GST_CLOCK_TIME_IS_VALID (min_dts) && min_dts != 0) { /* Unless the user has explicitly requested another seek, perform an @@ -7549,7 +7553,7 @@ gst_qtdemux_process_adapter (GstQTDemux * demux, gboolean force) } /* in MSS we need to expose the pads after the first moof as we won't get a moov */ - if (demux->mss_mode && !demux->exposed) { + if (demux->variant == VARIANT_MSS_FRAGMENTED && !demux->exposed) { QTDEMUX_EXPOSE_LOCK (demux); qtdemux_expose_streams (demux); QTDEMUX_EXPOSE_UNLOCK (demux); diff --git a/subprojects/gst-plugins-good/gst/isomp4/qtdemux.h b/subprojects/gst-plugins-good/gst/isomp4/qtdemux.h index ad150727c0..830ed2fda5 100644 --- a/subprojects/gst-plugins-good/gst/isomp4/qtdemux.h +++ b/subprojects/gst-plugins-good/gst/isomp4/qtdemux.h @@ -65,6 +65,19 @@ enum QtDemuxState QTDEMUX_STATE_BUFFER_MDAT /* Buffering the mdat atom */ }; +typedef enum { + /* Regular behaviour */ + VARIANT_NONE, + + /* We're working with a MediaSource Extensions ISO BMFF Bytestream. */ + VARIANT_MSE_BYTESTREAM, + + /* We're working with a smoothstreaming fragment. + * Mss doesn't have 'moov' or any information about the streams format, + * requiring qtdemux to expose and create the streams */ + VARIANT_MSS_FRAGMENTED, +} Variant; + struct _GstQTDemux { GstElement element; @@ -142,10 +155,7 @@ struct _GstQTDemux { guint32 segment_seqnum; - /* flag to indicate that we're working with a smoothstreaming fragment - * Mss doesn't have 'moov' or any information about the streams format, - * requiring qtdemux to expose and create the streams */ - gboolean mss_mode; + Variant variant; /* Set to TRUE if the incoming stream is either a MSS stream or * a Fragmented MP4 (containing the [mvex] atom in the header) */