mpegtsdemux: Allow deactivation of programs to be delayed

When changing programs, the order of events needs to be the following:
* add pads from new program
* send EOS on old pads
* remove old pads
* emit 'no-more-pads'

Previously tsdemux was not doing that, and was first deactivating and
removing old pads before adding new ones.

We fix this by allowing subclasses of mpegtsbase to be able to handle
themselves the deactivation of programs. In this case tsdemux will
properly deactivate it once it has activated the new program.

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

4 files changed, 68 insertions, 7 deletions

diff --git a/gst/mpegtsdemux/mpegtsbase.c b/gst/mpegtsdemux/mpegtsbase.c
index d46a47e94..3dcde601b 100644
--- a/gst/mpegtsdemux/mpegtsbase.c
+++ b/gst/mpegtsdemux/mpegtsbase.c
@@ -74,6 +74,8 @@ static void mpegts_base_get_property (GObject * object, guint prop_id,
     GValue * value, GParamSpec * pspec);
 
 static void mpegts_base_free_program (MpegTSBaseProgram * program);
+static void mpegts_base_deactivate_program (MpegTSBase * base,
+    MpegTSBaseProgram * program);
 static gboolean mpegts_base_sink_activate (GstPad * pad, GstObject * parent);
 static gboolean mpegts_base_sink_activate_mode (GstPad * pad,
     GstObject * parent, GstPadMode mode, gboolean active);
@@ -105,12 +107,21 @@ _extra_init (void)
 G_DEFINE_TYPE_WITH_CODE (MpegTSBase, mpegts_base, GST_TYPE_ELEMENT,
     _extra_init ());
 
+/* Default implementation is that mpegtsbase can remove any program */
+static gboolean
+mpegts_base_can_remove_program (MpegTSBase * base, MpegTSBaseProgram * program)
+{
+  return TRUE;
+}
+
 static void
 mpegts_base_class_init (MpegTSBaseClass * klass)
 {
   GObjectClass *gobject_class;
   GstElementClass *element_class;
 
+  klass->can_remove_program = mpegts_base_can_remove_program;
+
   element_class = GST_ELEMENT_CLASS (klass);
   element_class->change_state = mpegts_base_change_state;
 
@@ -411,6 +422,16 @@ mpegts_base_free_program (MpegTSBaseProgram * program)
 }
 
 void
+mpegts_base_deactivate_and_free_program (MpegTSBase * base,
+    MpegTSBaseProgram * program)
+{
+  GST_DEBUG_OBJECT (base, "program_number : %d", program->program_number);
+
+  mpegts_base_deactivate_program (base, program);
+  mpegts_base_free_program (program);
+}
+
+static void
 mpegts_base_remove_program (MpegTSBase * base, gint program_number)
 {
   GST_DEBUG_OBJECT (base, "program_number : %d", program_number);
@@ -472,7 +493,7 @@ mpegts_base_program_add_stream (MpegTSBase * base,
   return bstream;
 }
 
-void
+static void
 mpegts_base_program_remove_stream (MpegTSBase * base,
     MpegTSBaseProgram * program, guint16 pid)
 {
@@ -771,6 +792,7 @@ mpegts_base_apply_pat (MpegTSBase * base, GstMpegtsSection * section)
   }
 
   if (old_pat) {
+    MpegTSBaseClass *klass = GST_MPEGTS_BASE_GET_CLASS (base);
     /* deactivate the old table */
     GST_LOG ("Deactivating old Program Association Table");
 
@@ -791,8 +813,10 @@ mpegts_base_apply_pat (MpegTSBase * base, GstMpegtsSection * section)
       GST_INFO_OBJECT (base, "PAT removing program 0x%04x 0x%04x",
           patp->program_number, patp->network_or_program_map_PID);
 
-      mpegts_base_deactivate_program (base, program);
-      mpegts_base_remove_program (base, patp->program_number);
+      if (klass->can_remove_program (base, program)) {
+        mpegts_base_deactivate_program (base, program);
+        mpegts_base_remove_program (base, patp->program_number);
+      }
       /* FIXME: when this happens it may still be pmt pid of another
        * program, so setting to False may make it go through expensive
        * path in is_psi unnecessarily */
@@ -854,6 +878,7 @@ mpegts_base_apply_pmt (MpegTSBase * base, GstMpegtsSection * section)
 
   /* If the current program is active, this means we have a new program */
   if (old_program->active) {
+    MpegTSBaseClass *klass = GST_MPEGTS_BASE_GET_CLASS (base);
     old_program = mpegts_base_steal_program (base, program_number);
     program = mpegts_base_new_program (base, program_number, section->pid);
     program->patcount = old_program->patcount;
@@ -861,8 +886,12 @@ mpegts_base_apply_pmt (MpegTSBase * base, GstMpegtsSection * section)
         GINT_TO_POINTER (program_number), program);
 
     /* Desactivate the old program */
-    mpegts_base_deactivate_program (base, old_program);
-    mpegts_base_free_program (old_program);
+    /* FIXME : THIS IS BREAKING THE STREAM SWITCHING LOGIC !
+     *  */
+    if (klass->can_remove_program (base, old_program)) {
+      mpegts_base_deactivate_program (base, old_program);
+      mpegts_base_free_program (old_program);
+    }
     initial_program = FALSE;
   } else
     program = old_program;
diff --git a/gst/mpegtsdemux/mpegtsbase.h b/gst/mpegtsdemux/mpegtsbase.h
index c427bd711..fc0c6512c 100644
--- a/gst/mpegtsdemux/mpegtsbase.h
+++ b/gst/mpegtsdemux/mpegtsbase.h
@@ -174,6 +174,10 @@ struct _MpegTSBaseClass {
   void (*program_started) (MpegTSBase *base, MpegTSBaseProgram *program);
   /* program_stopped gets called when pat no longer has program's pmt */
   void (*program_stopped) (MpegTSBase *base, MpegTSBaseProgram *program);
+  /* Whether mpegtbase can deactivate/free a program or whether the subclass will do it
+   * If the subclass responds TRUE, it should call mpegts_base_deactivate_and_free_program()
+   * when it wants to remove it */
+  gboolean (*can_remove_program) (MpegTSBase *base, MpegTSBaseProgram *program);
 
   /* stream_added is called whenever a new stream has been identified */
   void (*stream_added) (MpegTSBase *base, MpegTSBaseStream *stream, MpegTSBaseProgram *program);
@@ -222,9 +226,8 @@ mpegts_base_handle_seek_event(MpegTSBase * base, GstPad * pad, GstEvent * event)
 
 G_GNUC_INTERNAL gboolean gst_mpegtsbase_plugin_init (GstPlugin * plugin);
 
-G_GNUC_INTERNAL void mpegts_base_program_remove_stream (MpegTSBase * base, MpegTSBaseProgram * program, guint16 pid);
+G_GNUC_INTERNAL void mpegts_base_deactivate_and_free_program (MpegTSBase *base, MpegTSBaseProgram *program);
 
-G_GNUC_INTERNAL void mpegts_base_remove_program(MpegTSBase *base, gint program_number);
 G_END_DECLS
 
 #endif /* GST_MPEG_TS_BASE_H */
diff --git a/gst/mpegtsdemux/tsdemux.c b/gst/mpegtsdemux/tsdemux.c
index fb8e6ac34..aac68f2f9 100644
--- a/gst/mpegtsdemux/tsdemux.c
+++ b/gst/mpegtsdemux/tsdemux.c
@@ -282,6 +282,9 @@ static void
 gst_ts_demux_program_started (MpegTSBase * base, MpegTSBaseProgram * program);
 static void
 gst_ts_demux_program_stopped (MpegTSBase * base, MpegTSBaseProgram * program);
+static gboolean
+gst_ts_demux_can_remove_program (MpegTSBase * base,
+    MpegTSBaseProgram * program);
 static void gst_ts_demux_reset (MpegTSBase * base);
 static GstFlowReturn
 gst_ts_demux_push (MpegTSBase * base, MpegTSPacketizerPacket * packet,
@@ -379,6 +382,7 @@ gst_ts_demux_class_init (GstTSDemuxClass * klass)
   ts_class->push_event = GST_DEBUG_FUNCPTR (push_event);
   ts_class->program_started = GST_DEBUG_FUNCPTR (gst_ts_demux_program_started);
   ts_class->program_stopped = GST_DEBUG_FUNCPTR (gst_ts_demux_program_stopped);
+  ts_class->can_remove_program = gst_ts_demux_can_remove_program;
   ts_class->stream_added = gst_ts_demux_stream_added;
   ts_class->stream_removed = gst_ts_demux_stream_removed;
   ts_class->seek = GST_DEBUG_FUNCPTR (gst_ts_demux_do_seek);
@@ -1760,6 +1764,24 @@ gst_ts_demux_flush_streams (GstTSDemux * demux, gboolean hard)
     gst_ts_demux_stream_flush (walk->data, demux, hard);
 }
 
+static gboolean
+gst_ts_demux_can_remove_program (MpegTSBase * base, MpegTSBaseProgram * program)
+{
+  GstTSDemux *demux = GST_TS_DEMUX (base);
+
+  /* If it's our current active program, we return FALSE, we'll deactivate it
+   * ourselves when the next program gets activated */
+  if (demux->program == program) {
+    GST_DEBUG
+        ("Attempting to remove current program, delaying until new program gets activated");
+    demux->previous_program = program;
+    demux->program_number = -1;
+    return FALSE;
+  }
+  return TRUE;
+}
+
+
 static void
 gst_ts_demux_program_started (MpegTSBase * base, MpegTSBaseProgram * program)
 {
@@ -1789,6 +1811,11 @@ gst_ts_demux_program_started (MpegTSBase * base, MpegTSBaseProgram * program)
       TSDemuxStream *stream = (TSDemuxStream *) tmp->data;
       activate_pad_for_stream (demux, stream);
     }
+    if (demux->previous_program) {
+      GST_DEBUG ("Deactivating previous program");
+      mpegts_base_deactivate_and_free_program (base, demux->previous_program);
+      demux->previous_program = NULL;
+    }
     gst_element_no_more_pads ((GstElement *) demux);
   }
 }
diff --git a/gst/mpegtsdemux/tsdemux.h b/gst/mpegtsdemux/tsdemux.h
index b689412a2..b416733d7 100644
--- a/gst/mpegtsdemux/tsdemux.h
+++ b/gst/mpegtsdemux/tsdemux.h
@@ -65,6 +65,8 @@ struct _GstTSDemux
 
   /*< private >*/
   MpegTSBaseProgram *program;	/* Current program */
+  MpegTSBaseProgram *previous_program; /* Previous program, to deactivate once
+					* the new program becomes active */
 
   /* segments to be sent */
   GstSegment segment;