[Bug 791858] GstTocSetter: proposal to help standardize elements behaviour

[GStreamer (GNOME Bugzilla)]

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

--- Comment #9 from fengalin at free.fr ---
Created attachment 366679
  --> https://bugzilla.gnome.org/attachment.cgi?id=366679&action=edit
Add TOC selection mode

This is an implementation of the basic proposal above. This raises additional
questions and comments.

1. Naming
---------

I used the terms UPSTREAM for the TOC received from events and USER for the TOC
defined by the application. Whenever possible, I added a precision that the
upstream TOC was the one received from events. Is this acceptable or should I
change these to EVENT and APPLICATION?


2. Defaults
-----------

I decided to use GST_TOC_SELECTION_MODE_UPSTREAM as the default to reflect
current behaviour for elements that implement the GstTocSetter interface. If
the user defines a TOC using gst_toc_setter_set_toc(), the mode automatically
switches to GST_TOC_SELECTION_MODE_USER. If the user calls
gst_toc_setter_set_toc() with a NULL TOC, the mode automatically switches to
GST_TOC_SELECTION_MODE_NONE. Users can force the TOC selection mode using the
dedicated setter. The documentation is explicit about these.

Is this OK?


3. Comment in GstTocSetter's description
----------------------------------------

AFAICT, the following comment in GstTocSetter's description doesn't match
current behaviour for some implementers:

 * Elements implementing the #GstTocSetter interface can extend existing TOC
 * by getting extend UID for that (you can use gst_toc_find_entry() to retrieve
it)
 * with any TOC entries received from downstream.

In wavenc & flacenc, the upstream TOC is not accessible from the GstTocSetter
interface. In matroskamux, the upstream TOC replaces the user TOC if it was set
before, so it matches the description, however the user can't define a TOC
beforehand.

To standardize the GstTocSetter further, I think we should add an upstream_toc
field to the GstTocSetter's internal data (and the associated setters/getters).
The elements would use this field instead of defining their own and the
application would be able to access the upstream TOC without the need to define
a pad probe. The above comment would be replaced by an explanation about the
upstream / user TOC getters.

Shall I go on with these modifications?


4. gst_toc_setter_reset()
-------------------------

The documentation for this function states:

 * Reset the internal TOC. Elements should call this from within the
 * state-change handler.

As expected, it resets the user provided TOC. This is done in different states
transitions depending on the element:

- matroskamux: calls it in Paused to Ready. Since the upstream TOC and the user
TOC as stored in the same field, this makes sense, but it defeats the idea of a
persistent user TOC.
- wavenc: calls it in Ready to NULL.
- flacenc is an GstAudioEncoder and calls gst_toc_setter_reset() in stop()
which is called when the sink pad is deactivated (so I guess that must be Ready
to NULL).

I think we should be more specific about the transition the element should call
gst_toc_setter_reset() and I would use Ready to NULL in order to comply with
the persistent nature of the user TOC.

If we implement the proposal in (3), we'd probably want to reset the upstream /
user TOCs during different transitions. I guess that would be:
- Paused to Ready: reset the upstream TOC.
- Ready to NULL: reset the user TOC.

Is that correct?

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