[Bug 795424] qtdemux: Add MSE-style flush

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=795424

--- Comment #18 from Alicia Boya García <aboya at igalia.com> ---
Review of attachment 371339:
 --> (https://bugzilla.gnome.org/review?bug=795424&attachment=371339)

::: gst/isomp4/qtdemux.c
@@ +1526,3 @@
   gst_event_set_seqnum (event, seqnum);
+  qtdemux->offset_seek_seqnum = seqnum;
+  qtdemux->propagate_flush = TRUE;

> This is the only place where it is set to TRUE, in the user flushing case

Actually that's not the case. That line of code belongs to
gst_qtdemux_do_push_seek(), which is for handling seeks.

As the comment in qtdemux.h explains, qtdemux->propagate_flush only applies to
flushes that are a consequence of a seek. User caused flushes are always
propagated downstream.

Let's review the three cases where qtdemux can receive a flush.

1) The demuxer receives a TIME SEEK event from downstream. The TIME SEEK event
is consumed and gst_qtdemux_do_push_seek() is called. qtdemux finds what byte
position in the file corresponds to the requested time position using the
sample table and sends a BYTE SEEK upstream. Upstream rewinds the file and
sends a flush downstream, followed by a BYTE segment and then the new data.
qtdemux takes advantage of this to propagate the flush downstream so that
GStreamer stops decoding the old frames.

2) The demuxer attempts to read a file where the mdat precedes the moov. At the
beginning, the demuxer finds out about this and calls qtdemux_seek_offset() to
skip the mdat (whose size is known). This sends a BYTE SEEK upstream too, which
again causes a flush, segment and new data to arrive at qtdemux. In this case
qtdemux does not have any need to propagate the flush downstream, so it
doesn't. This is the case where propagate_flush = FALSE.

3) The user sends a flush on their own to the demuxer, not caused by any seek.
This is the new use case introduced by this patch. Because of how the MP4
format is designed, this use case is only useful for fragmented media. In it,
the fragment currently being parsed is discarded (this includes its sample
table) and a new one is expected. [In lower level terms, the flush instructs
the demuxer to start reading a MP4 element from the beginning
(QTDEMUX_STATE_INITIAL), clears all buffered data and discards the sample table
of each track; but header data obtained from the moov -- if any, remains.] In
this case the flush is always propagated, regardless of the propagate_flush
property.

gst_qtdemux_handle_sink_event is able to tell apart cases 1 and 2 from case 3
by looking at the seqnum of the flush event and comparing it to
demux->offset_seek_seqnum.

Regarding case 2, I have some questions:

1. Would anything bad happen if I just propagated the flush downstream?
2. Couldn't we just remove the GST_SEEK_FLAG_FLUSH flag and just skip the flush
completely? I don't think we are doing anything useful on response to it and
we're not propagating it either.

I tried to play safe by keeping the current behavior, but I would not be
against simplifying it out.

::: gst/isomp4/qtdemux.h
@@ -196,2 +196,5 @@
    * detect incoming FLUSH events corresponding to that */
   guint32 offset_seek_seqnum;
+  /* Whether the FLUSH events received in consequence of a seek
+   * sent upstream should be propagated downstream. */
+  gboolean propagate_flush;

Other flushes are always propagated. It could be better clarified.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.