Some support for transcoding would also be nice, so that chapters from a DVD would automagically turn into mkv chapters for instance.  Maybe by using a (standardized) "custom" chapter event sent at start (end?) of a chapter, so that downstream can pick this up and also start a new chapter.

So for transcoding we would need a CHAPTERS event or tag that the muxer can handle then...

Another thing that we should support is metadata of the chapters, like the name of the chapter. I can't think of more right now so maybe it's enough to support a name for chapters... otherwise a GstStructure for metadata might also make sense ;)

The implementation could be an interface that playbin and/or decodebin are also implementing and proxy to the demuxer if the demuxer supports it.

Also the implementation should probably register a central GstFormat for seeking/querying the chapter (GST_FORMAT_CHAPTER).

OTOH this could also be implemented as a new GstQuery... Mark, Bastien what do you think? ;)

The GstQuery could then contain a GstStructure, containing a GstArray which contains GstStructures containing the TOC information.

The query could be called TOC, gst_query_new_toc() would take a variable number of GstQueryTocEntry and gst_query_parse_toc() would return a NULL-terminated array of GstQueryTocEntry.

GstQueryTocEntry would contain various information about a chapter/track, like name, duration, .... and a GstStructure for additional information. Fortunately we can have nested GstStructure nowadays :)

This then would make it possible to use the TOCs with the standard GStreamer techniques: queries for getting information, seeking in a new format for selecting a specific chapter/track, ...

As this had to go to core it should be media independend and I guess this is the case here as one could, for example, implement an element parsing URI lists and exporting them via the TOC query.

Only question is, what the terminology for such a TOC entry should be and how the new GstFormat could be called. Is it a chapter? A track? A TOC entry?

IMHO this would be the preferred implementation.

Some random thoughts:

 - why have a TOC query vs. posting a TOC message on the bus?
   (query would take same path as seek event; but it's inconsistent
   with how we announce tags; if it was a query, would we also have
   a TOC-changed message then?)

 - we'd need to allow for tree-like structures to be flexible, right?
   Is this easy-enough to do in a way that doesn't make it extremely
   painful to handle in apps? (we could predefine fixed hierarchy
   schemes, like title:chapter or disc:tracks with a bunch of utility
   functions for each)

 - do we need a 'TOC' at all? Why not just put it into the normal tags,
   possibly with utility functions for easier parsing? (see e.g. the
   rather messy and basic, and also commented out, GST_TAG_CDDA_TRACK_TAGS
   stuff in cddabasesrc)

I like the idea of using a GstMessage instead of queries... I wonder why I didn't think about it :)

Another idea for the complete implementation would be to use a GstMessage containing the complete TOC. It would be posted by elements if they know the TOC and contains a GstStructure representing the complete TOC. Inside this GstStructure there will be an array of GstStructures which represent a track/chapter/whatever which contain all kinds of metadata and could also contain another array of GstStructures.
This way we can represent a tree-like structure of chapters/tracks.

Every track/chapter GstStructure would contain a GstFormat and a start/stop position in this format. Applications would then listen for that message, parse it to their needs and can use normal seeks to select a specific chapter.

After thinking about it a bit more and talking to Wim my current plan is like this.

We get a new event, message and query type for the TOC. These will all contain a GstStructure which represents the TOC.

This GstStructure contains information about the complete file and should contain an array of GstStructure for all "subparts" and these can again contain more GstStructures to create a tree-like structure.

The names for these GstStructure can be "chapter" or "edition" and all child structures of a parent must have the same name. edition represents alternatives, chapters represent parts. So let's say you have a movie separated into 5 chapters and for the last chapter there exists an alternative version then you would create the following structure:

                     toc
           edition1---|----edition2
        /   |                 /   |
   chapter1 chapter2 ...     ch1 ......  chapter5b

Another example: An album encoded to a single file would have the toc structure containing an array of chapters.
Or a DVD with angles (assuming angles are per title) would be a TOC containing the titles as chapters and the titles containing editions for the angles.


Now every of these structures should contain some fields. The top level toc structure probably doesn't need to contain any fields but we might want to include the taglist containing tags which are valid for the complete file or the complete duration or something.

The chapters and editions must contain a unique identifier (I'd suggest using a int64 for reasons explained later) and should also contain a name that can be displayed to the user, the taglist with all tags specific to the chapter/edition, the duration (including all durations of the child elements) and a GstFormat for the duration.

Applications can show the user some kind of UI for this TOC tree and allow them to select a specific chapter, etc.


Now the application needs a way to choose a specific chapter/edition or a range of chapters/editions. For ranges IMHO it's enough to allow the application to choose a start and stop chapter/edition at the same level or in the subtree of the start chapter of the TOC tree.
The demuxer/source then has to play the complete subtree of every chapter/edition and then stop playback.

I'd suggest that we invent a new GstFormat (GST_FORMAT_CHAPTER) for this. Applications can then send a seek event in this format to the demuxer, the start and stop position of the seek are the unique identifiers for the specific chapter/edition. The demuxer/source will then go into some kind of "chapter" mode and all following seek events, position/duration queries in other formats will be relative to the currently playing chapters/editions. To get the old behaviour back again a pre-defined UID is used for seeking.

To provide the application about some informations of the currently playing chapter/edition I'd suggest using a new tag type ("chapter"). This will contain the UID of the currently playing chapter/edition and will be sent by the demuxer/source if it's in "chapter mode". This way the application always knows at which point we're currently playing.

The demuxer/source should also send chapter/edition specific tags at the time a chapter/edition is started.



Does anybody see issues with this proposal? Otherwise I'd start to write a prototype implementation for the matroska element (as matroska has the probably most complex chapter/editions system out there it should be the perfect test ;) )

One issue I noticed is with the seeking.

As an application you usually want to select an element of the TOC tree or a few elements with the same parent. Then playback starts and you might want to skip to the next chapter but continue playback of what was specified before.
For this I'd suggest:

  GST_SEEK_TYPE_SET in GST_FORMAT_CHAPTER will set a new chapter selection that will be played, i.e. the full subtree of all selected elements with the same parent element.
  
  GST_SEEK_TYPE_CUR in GST_FORMAT_CHAPTER will keep the previous selection but will seek to the specified chapter.

  GST_SEEK_TYPE_CUR in other formats will be relative to the chapter selection done with GST_SEEK_TYPE_SET in GST_FORMAT_CHAPTER and relative to the current playback position.

  GST_SEEK_TYPE_SET in other formats after a GST_SEEK_TYPE_SET in GST_FORMAT_CHAPTER will be handled relative to the selected chapters.

Probably stating the obvious, but if the (new) event mentioned above makes it downstream for elements there to figure out what is going on (e.g. muxers), then that should be fine for transcoding :)

[Or maybe not that obvious, IIRC there are (often?) "global tags" that demuxers only announce as messages, and that therefore do not give downstream a chance ...]

(In reply to comment #7)
> Does anybody see issues with this proposal?

Hello Sebastian, I have an issue with this item:

> This GstStructure contains information about the complete file and should
> contain an array of GstStructure for all "subparts" and these can again contain
> more GstStructures to create a tree-like structure.

In the case of a DVD this can be an expensive operation to collect all the information to create the 1 tree for whole the disc, because the navigation data is spread across the disc, and disc access can be slow...

Accept from that the information about other titles on the disc is not interesting for the player and there will be much information in the tree that can't be translated to useful information for the end-user. So maybe it is the best to send a TOC only for the current playing title (angles/chapters), and if the user play's another title on this DVD a new TOC is send....

I am not sure if seeking to editions fits the seeking metaphor. One seeks in a stream and chapters are sort of named points in time in that stream. Editions are more like a separate alternate stream.

As a sidenote that might be useful for wavparse to. It can have loop-markers (loop-start/end) or cue lists. Where cue-lists are just named position markers, lopp-markers have even some special semantics. Not sure if we can easily mark special purpose TOC entries?

(In reply to comment #9)
> Probably stating the obvious, but if the (new) event mentioned above makes it
> downstream for elements there to figure out what is going on (e.g. muxers),
> then that should be fine for transcoding :)
> 
> [Or maybe not that obvious, IIRC there are (often?) "global tags" that demuxers
> only announce as messages, and that therefore do not give downstream a chance
> ...]

If elements are using gst_element_found_tags or _found_tags_for_pad the tags will be sent as message and event. The only problem with some elements is, that they send tags before a first NEWSEGMENT event which will make the tags-event be dropped.

(In reply to comment #10)
> (In reply to comment #7)
> > Does anybody see issues with this proposal?
> 
> Hello Sebastian, I have an issue with this item:
> 
> > This GstStructure contains information about the complete file and should
> > contain an array of GstStructure for all "subparts" and these can again contain
> > more GstStructures to create a tree-like structure.
> 
> In the case of a DVD this can be an expensive operation to collect all the
> information to create the 1 tree for whole the disc, because the navigation
> data is spread across the disc, and disc access can be slow...
> 
> Accept from that the information about other titles on the disc is not
> interesting for the player and there will be much information in the tree that
> can't be translated to useful information for the end-user. So maybe it is the
> best to send a TOC only for the current playing title (angles/chapters), and if
> the user play's another title on this DVD a new TOC is send....

That would be possible too of course... we could make the TOCs for DVDs be partial. I.e. first you will have just all titles, when the first title is played an updated TOC with all chapters/angles of this title added will be sent, etc. For this to work good we just need an "updated" flag in the top-level TOC structure. Does this sound good for you?

(In reply to comment #11)
> I am not sure if seeking to editions fits the seeking metaphor. One seeks in a
> stream and chapters are sort of named points in time in that stream. Editions
> are more like a separate alternate stream.

The problem is, that these alternatives are not always at the top of the TOC tree. DVD angles for example are specific to a title AFAIK (so you have TOC->Title(as chapters)->Angle(as editions)->Chapters(as chapters).

In Matroska and all other formats I know they're fortunately top-level only...

> As a sidenote that might be useful for wavparse to. It can have loop-markers
> (loop-start/end) or cue lists. Where cue-lists are just named position markers,
> lopp-markers have even some special semantics. Not sure if we can easily mark
> special purpose TOC entries?

Well, it's a GstStructure... we can always add new things to it without breaking compatibility. I've also thought about "hidden" TOC entries which are not shown to the user but could be seeked to by a format specific mechanism (DVD menus, Matroska menus or "scripts", ...)

Adding myself to cc list.

*** Bug 572049 has been marked as a duplicate of this bug. ***

What is the progress of this bug. When it is supposed to be implemented?

This missing feature in gstreamer is the main reason I can't use rythmbox nor elisa/moovida. 
Cuesheets & single-file-per-album flacs are the only practical to approach the "classical music problem" where gaps are not acceptable.

> This missing feature in gstreamer is the main reason I can't use rythmbox nor
> elisa/moovida. Cuesheets & single-file-per-album flacs are the only practical
> to approach the "classical music problem" where gaps are not acceptable.

Well, nothing stops any of these music players to implement this themselves, esp. given that most of these players do some sort of playlist parsing/handling already. A few years back I wrote a music player for GStreamer-0.8 that handled these perfectly (that's why I wrote it actually), and it wasn't that much effort, so I don't see why rb or cowplayer can't do that as well if they really want to.

(This is not to say that this is not a useful feature to get into GStreamer, but there's no need to block on this. And it's important to get the API right IMO).

I also recently encountered this problem when it comes to multiple subtunes/subtracks within single files, common in several video game music formats and various tracked formats.

The gme plugin only plays track 0 right now, but, different from DVD chapters, and perhaps also different from classical music, I would think it more logical to have the different subtracks show up as different songs. This would also work nicely in most players, where one can make individual playlists of different subtunes and so on. 

This would not be possible as there is no way to send information on which subtune to play. (Or if it is, please correct me!)

Created attachment 166065 [details] [review]
Generic TOC support

Created attachment 166066 [details] [review]
Add TOC event

Created attachment 166067 [details] [review]
Add TOC message

Created attachment 166068 [details] [review]
GstToc section in the docs

Created attachment 166069 [details] [review]
Check tests for the GstToc

Hey guys,

As part of my GSoC 2010 work I've made patches for GStreamer core which implement generic table of contents.

Patches claim to introduce API in 0.10.29 instead of 0.10.31.

Also some minor comments I've already sent you by mail:

- The GQuark registration. Use g_quark_from_static_string() instead of
g_quark_from_string() please.
- gst_toc_entry_new(): Add a g_return_val_if_fail (uid != NULL, NULL);
and g_return_val_if_fail (stop == -1 || start <= stop, NULL); to prevent
invalid values.
- gst_toc_entry_structure_new(): Set the type as enum but set the
format as uint and cast. Otherwise you can't support custom GstFormats.
The type setting is correct.
- gst_toc_entry_from_structure(): The same as above with the GstFormat
- gst_toc_entry_to_structure() and _gst_toc_to_structure(): Create the
GstArray a single time, append all list values and then set it in the
structure instead of creating a copy of the array every time, appending
a single entry to the copy and then replace the original with the
modified copy.
- Add to the docs, that the UID of an entry must be persistent for
updates and that it should be handled as "opaque" value without meaning
(e.g. apps shouldn't assume the /editionX/chapterY/chapter/Z structure,
other demuxers could do something else)


Other than that I really like the patches. Would be nice if someone else would review them too and comment on them :)

This approach should support:
- Generic chapters/editions like in Matroska
- Cuesheets (FLAC, WAV)
- Playlists (You could write e.g. a m3ubin that uses this and internally creates pipelines for the streams)

Is there anything else we would like to support?

Oops, some of the files I made patches from were outdated a bit, I'll submit proper patches in a few minutes, sorry.

Created attachment 166073 [details] [review]
Generic TOC support

Created attachment 166074 [details] [review]
Add TOC event

Created attachment 166075 [details] [review]
Add TOC message

Created attachment 166076 [details] [review]
GstToc section in the docs

Created attachment 166077 [details] [review]
Check tests for the GstToc

Here is a git repository with updated GstToc:

git://gitorious.org/gstreamer-chapters/gstreamer-core.git

I discussed this with Tim yesterday and there are some problems with this.

a) When muxing something with chapters/editions, you might want to use sink_0 for chapter 1 of the video and sink_1 for chapter 2 of the video. So there needs to be some way, to specify pads for chapters in the GstTocEntry.

b) You might want to attach other information to tocs/toc entries, like a description ("this is the high resolution version") or something to specify the default edition etc. For this a GstStructure should be added to the toc and toc entries.

c) If you have a playlist "demuxer", followed by the matroska demuxer for example, they're both exposing a TOC. But the application has no way to use both TOCs and it gets even worse if there are more TOC elements next to the matroska demuxer. For this we need quite some changes:
  - A TOC query, which allows to query the complete TOC but also allows elements to query "their position" in the TOC so they can extent the TOC somehow
  - A TOC select event (upstream), which is like a seek event but for TOC UIDs (this gets rid of the start/stop/format stuff from the entries). This is required to allow seeking in these nested/cascaded TOCs unfortunatenly. Normal seeks would only seek in the selected TOC parts.


So there's still some stuff to do to get generic enough to support all important use cases.

> a) When muxing something with chapters/editions, you might want to use sink_0
> for chapter 1 of the video and sink_1 for chapter 2 of the video. So there
> needs to be some way, to specify pads for chapters in the GstTocEntry.

Well, there are two ways to go, I think: to put such info right into the GstTocEntry, or store it somewhere in external structure. In first case we can store pointer to GstPad, but this information will be useless for other elements in pipeline I think, and especially for applications. Or we can store assotiations between pads and GstTocEntri'es in some external structure, like GHashTable. And muxer can operate with it in any way. Any other ideas?

> b) You might want to attach other information to tocs/toc entries, like a
> description ("this is the high resolution version") or something to specify the
> default edition etc. For this a GstStructure should be added to the toc and toc
> entries.

Would there be any predefined fields in this GstStructure or not? Maybe description, or default entry, or start entry, etc?

> c) If you have a playlist "demuxer", followed by the matroska demuxer for
> example, they're both exposing a TOC. But the application has no way to use
> both TOCs and it gets even worse if there are more TOC elements next to the
> matroska demuxer. For this we need quite some changes:
>   - A TOC query, which allows to query the complete TOC but also allows
> elements to query "their position" in the TOC so they can extent the TOC
> somehow

I suggest to add TOC query (which contains query mode - whether to get full TOC or to get position to insert other TOC entries), which will return UID of the parent GstTocEntry (where to insert another TOC elements) and the type (chapter or edition) of GstTocEntry which can be inserted in that parent entry (for example, we return 0 for UID and EDITION for the type, which means that other entries should be inserted right into the root TOC and these entries should be editions) in case of position request mode, and full TOC in case of TOC request mode.

>   - A TOC select event (upstream), which is like a seek event but for TOC UIDs
> (this gets rid of the start/stop/format stuff from the entries). This is
> required to allow seeking in these nested/cascaded TOCs unfortunatenly. Normal
> seeks would only seek in the selected TOC parts.

I think that a new TOC seek event (maybe GST_EVENT_TOC_SEEK) with the seeking UID would be ok, isn't it?

(In reply to comment #34)
> > a) When muxing something with chapters/editions, you might want to use sink_0
> > for chapter 1 of the video and sink_1 for chapter 2 of the video. So there
> > needs to be some way, to specify pads for chapters in the GstTocEntry.
> 
> Well, there are two ways to go, I think: to put such info right into the
> GstTocEntry, or store it somewhere in external structure. In first case we can
> store pointer to GstPad, but this information will be useless for other
> elements in pipeline I think, and especially for applications. Or we can store
> assotiations between pads and GstTocEntri'es in some external structure, like
> GHashTable. And muxer can operate with it in any way. Any other ideas?

Put them directly into the GstTocEntry. Otherwise things will become quite complicated.

> > b) You might want to attach other information to tocs/toc entries, like a
> > description ("this is the high resolution version") or something to specify the
> > default edition etc. For this a GstStructure should be added to the toc and toc
> > entries.
> 
> Would there be any predefined fields in this GstStructure or not? Maybe
> description, or default entry, or start entry, etc?

Yes, but I don't think we should worry about that right now.

> > c) If you have a playlist "demuxer", followed by the matroska demuxer for
> > example, they're both exposing a TOC. But the application has no way to use
> > both TOCs and it gets even worse if there are more TOC elements next to the
> > matroska demuxer. For this we need quite some changes:
> >   - A TOC query, which allows to query the complete TOC but also allows
> > elements to query "their position" in the TOC so they can extent the TOC
> > somehow
> 
> I suggest to add TOC query (which contains query mode - whether to get full TOC
> or to get position to insert other TOC entries), which will return UID of the
> parent GstTocEntry (where to insert another TOC elements) and the type (chapter
> or edition) of GstTocEntry which can be inserted in that parent entry (for
> example, we return 0 for UID and EDITION for the type, which means that other
> entries should be inserted right into the root TOC and these entries should be
> editions) in case of position request mode, and full TOC in case of TOC request
> mode.

Well, I'd probably use a single TOC query type and it will always return the complete TOC but contains some kind of marker for the TOC Entry for the pad over which the query was received. That's probably a bit easier.

> >   - A TOC select event (upstream), which is like a seek event but for TOC UIDs
> > (this gets rid of the start/stop/format stuff from the entries). This is
> > required to allow seeking in these nested/cascaded TOCs unfortunatenly. Normal
> > seeks would only seek in the selected TOC parts.
> 
> I think that a new TOC seek event (maybe GST_EVENT_TOC_SEEK) with the seeking
> UID would be ok, isn't it?

Yes, but seek is no good metaphor for this IMHO. It's more a selection than a seek.


Another thing we probably want is a "GstTocSetter" interface, similar to the GstTagSetter interface. Which would then be used for muxers to set the TOC properly on them. Sending events from the application to the element is not really clean.

> > I suggest to add TOC query (which contains query mode - whether to get full TOC
> > or to get position to insert other TOC entries), which will return UID of the
> > parent GstTocEntry (where to insert another TOC elements) and the type (chapter
> > or edition) of GstTocEntry which can be inserted in that parent entry (for
> > example, we return 0 for UID and EDITION for the type, which means that other
> > entries should be inserted right into the root TOC and these entries should be
> > editions) in case of position request mode, and full TOC in case of TOC request
> > mode.
> 
> Well, I'd probably use a single TOC query type and it will always return the
> complete TOC but contains some kind of marker for the TOC Entry for the pad
> over which the query was received. That's probably a bit easier.
>


Just to be sured that I understood all the things correctly: some element receives TOC query, then it checks the pad over which that query was received and decides which TOC entry to mark as "extendable" (or some kind of that) and returns UID of that TOC entry with the full TOC. Is that right?

(In reply to comment #35)
> 
> Another thing we probably want is a "GstTocSetter" interface, similar to the
> GstTagSetter interface. Which would then be used for muxers to set the TOC
> properly on them. Sending events from the application to the element is not
> really clean.

Is that to set the toc on the right element? I wonder if applications would know on which element the should set the toc if there are multimple TocSetter elements in a transcoding pipleine ...

Created attachment 167092 [details] [review]
Generic TOC support

Created attachment 167093 [details] [review]
Add TOC and TOC select events

Created attachment 167094 [details] [review]
Add TOC message

Created attachment 167095 [details] [review]
Add TOC query

Created attachment 167096 [details] [review]
GstToc section in the docs

Created attachment 167097 [details] [review]
Check tests for the GstToc

Here is another set of patches. I added TOC query and TOC select event, changed GstToc and GstTocEntry acoordingly to discussed stuff, updated Check tests and docs. So, another thing I need to implement is a GstTocSetter interface. I will make it a bit later and let you know.

(In reply to comment #36)
> > > I suggest to add TOC query (which contains query mode - whether to get full TOC
> > > or to get position to insert other TOC entries), which will return UID of the
> > > parent GstTocEntry (where to insert another TOC elements) and the type (chapter
> > > or edition) of GstTocEntry which can be inserted in that parent entry (for
> > > example, we return 0 for UID and EDITION for the type, which means that other
> > > entries should be inserted right into the root TOC and these entries should be
> > > editions) in case of position request mode, and full TOC in case of TOC request
> > > mode.
> > 
> > Well, I'd probably use a single TOC query type and it will always return the
> > complete TOC but contains some kind of marker for the TOC Entry for the pad
> > over which the query was received. That's probably a bit easier.
> >
> 
> 
> Just to be sured that I understood all the things correctly: some element
> receives TOC query, then it checks the pad over which that query was received
> and decides which TOC entry to mark as "extendable" (or some kind of that) and
> returns UID of that TOC entry with the full TOC. Is that right?

Something like that, yes.

(In reply to comment #37)
> (In reply to comment #35)
> > 
> > Another thing we probably want is a "GstTocSetter" interface, similar to the
> > GstTagSetter interface. Which would then be used for muxers to set the TOC
> > properly on them. Sending events from the application to the element is not
> > really clean.
> 
> Is that to set the toc on the right element? I wonder if applications would
> know on which element the should set the toc if there are multimple TocSetter
> elements in a transcoding pipleine ...

It's also better to have an interface because sending events from app to element is not very clean. And additionally it allows the elements to see a difference between application set TOCs and TOCs sent by upstream elements. Usually you want to prefer the former.

(In reply to comment #38)
> Created an attachment (id=167092) [details] [review]
> Generic TOC support

There's no need for the format, start and stop values now with the TOC select event. You can remove them. But for the GstPad / gpointer issue we need to find a solution :(
gst_toc_entry_new() should probably take a pad parameter too.



Other than that this looks good now... But we should really try to implement this in the matroska demuxer/muxer first and in flacdec/flacenc and additionally implement a playlist element to see if this really fits with all the use cases.
Especially if a playlist element followed by the matroska demuxer really works :)


And then the pad selection might not be really optimal for muxers. Some muxers will require the same pads for the complete stream and then allow to structure that stream with a TOC (e.g. flac). Other muxers (e.g. matroska) could mux many different streams on many different pads and then structure/group them with a TOC. So the application really has to know what the element it uses needs.

(And in most cases the pad will be NULL, e.g. the complete stream/stream group I guess... for muxing at least)

(In reply to comment #46)

> And then the pad selection might not be really optimal for muxers. Some muxers
> will require the same pads for the complete stream and then allow to structure
> that stream with a TOC (e.g. flac). Other muxers (e.g. matroska) could mux many
> different streams on many different pads and then structure/group them with a
> TOC. So the application really has to know what the element it uses needs.

And for some use cases this has to be a list I think. So if you make it a GList we can at least get around the GstPad/gpointer problem... and add more complexity ;)

(In reply to comment #46)
> Other than that this looks good now... But we should really try to implement
> this in the matroska demuxer/muxer first and in flacdec/flacenc and
> additionally implement a playlist element to see if this really fits with all
> the use cases.

I have been following this progress with excitement, and I would just like to point out one use case which otherwise might be unnoticed.
Videogame music and tracked music often have many songs in one file.

Preferably, applications wouldn't view these as chapters, but as completely separate songs. I guess in that regard it would be vastly different from, say, an MKV file, or some other playlist.

Is this possible using this design?

(In reply to comment #48)
> (In reply to comment #46)
> > Other than that this looks good now... But we should really try to implement
> > this in the matroska demuxer/muxer first and in flacdec/flacenc and
> > additionally implement a playlist element to see if this really fits with all
> > the use cases.
> 
> I have been following this progress with excitement, and I would just like to
> point out one use case which otherwise might be unnoticed.
> Videogame music and tracked music often have many songs in one file.
> 
> Preferably, applications wouldn't view these as chapters, but as completely
> separate songs. I guess in that regard it would be vastly different from, say,
> an MKV file, or some other playlist.
> 
> Is this possible using this design?

Yes, we should find better names for the two TOC entry types. Something like "part" and "alternative" maybe...

> There's no need for the format, start and stop values now with the TOC select
> event. You can remove them.

I think that these values would be useful for some reasons:

1. To seek to appropriate time in TOC select event handling. Otherwise how can I get time for particular UID?

2. It would be useful for applications to display start/stop positions of the chapters/tracks.

Maybe leave them?

> gst_toc_entry_new() should probably take a pad parameter too.

So, I'll change gpointer to GList and add a pad parameter to gst_toc_entry(), ok?

Well, GstTocSetter interface is ready :) but I want to write some tests for it before. I need a bit of time for that.

> And then the pad selection might not be really optimal for muxers. Some muxers
> will require the same pads for the complete stream and then allow to structure
> that stream with a TOC (e.g. flac). Other muxers (e.g. matroska) could mux many
> different streams on many different pads and then structure/group them with a
> TOC. So the application really has to know what the element it uses needs.
> 
> (And in most cases the pad will be NULL, e.g. the complete stream/stream group
> I guess... for muxing at least)

You mean that one stream can be muxed from some pads with the TOC, then another stream can be muxed from another set of pads with the TOC, and finally we need to know, which pads in the TOC to which stream belong?

(In reply to comment #49)
> (In reply to comment #48)
> > (In reply to comment #46)
> Yes, we should find better names for the two TOC entry types. Something like
> "part" and "alternative" maybe...

We can open a voting for that :) It's really hard to find generic terms as in different formats it has different meanings. As for "alternative", it fits the best, I think. As for chapter/part/track type... Yeah, maybe the part is the most neutral term for that. I can recover from my head only a few generic terms for that, like "segment" or "section". Does anyone have other ideas?

(In reply to comment #50)
> > There's no need for the format, start and stop values now with the TOC select
> > event. You can remove them.
> 
> I think that these values would be useful for some reasons:
> 
> 1. To seek to appropriate time in TOC select event handling. Otherwise how can
> I get time for particular UID?

Well, you would put the UID into the TOC select event and the element would then handle everything needed, e.g. the conversion to time or something else. For a playlist element there wouldn't be any times for example.

> 2. It would be useful for applications to display start/stop positions of the
> chapters/tracks.

Right, that might be useful. But for that we could put something into the GstStructure because it's not something generic but only useful for some elements.

> > gst_toc_entry_new() should probably take a pad parameter too.
> 
> So, I'll change gpointer to GList and add a pad parameter to gst_toc_entry(),
> ok?

Not sure, before you work on this a second oppinion might be good to have :)

You could add a gst_toc_entry_new() without pad parameter, a gst_toc_entry_new_with_pad() with a single pad parameter and gst_toc_entry_new_with_pads() with a const GList* of pads. Or maybe only the first, I don't know

> Well, GstTocSetter interface is ready :) but I want to write some tests for it
> before. I need a bit of time for that.

Great :)

> > And then the pad selection might not be really optimal for muxers. Some muxers
> > will require the same pads for the complete stream and then allow to structure
> > that stream with a TOC (e.g. flac). Other muxers (e.g. matroska) could mux many
> > different streams on many different pads and then structure/group them with a
> > TOC. So the application really has to know what the element it uses needs.
> > 
> > (And in most cases the pad will be NULL, e.g. the complete stream/stream group
> > I guess... for muxing at least)
> 
> You mean that one stream can be muxed from some pads with the TOC, then another
> stream can be muxed from another set of pads with the TOC, and finally we need
> to know, which pads in the TOC to which stream belong?

Yes, for example.

(In reply to comment #52)
> (In reply to comment #50)
> > > There's no need for the format, start and stop values now with the TOC select
> > > event. You can remove them.
> > 
> > I think that these values would be useful for some reasons:
> > 
> > 1. To seek to appropriate time in TOC select event handling. Otherwise how can
> > I get time for particular UID?
> 
> Well, you would put the UID into the TOC select event and the element would
> then handle everything needed, e.g. the conversion to time or something else.
> For a playlist element there wouldn't be any times for example.

If I remove start/stop/format values, then element will have to store time data (start/stop if any) related to every UID somewhere in internal structure. And after receiving TOC select event element will look in that internal structure (and not in the TOC) to find positions for particalur UID. Is that right?

> > 2. It would be useful for applications to display start/stop positions of the
> > chapters/tracks.
> 
> Right, that might be useful. But for that we could put something into the
> GstStructure because it's not something generic but only useful for some
> elements.

Well, we can think of it after I'll take care for other stuff :)

(In reply to comment #53)
> (In reply to comment #52)
> > (In reply to comment #50)
> > > > There's no need for the format, start and stop values now with the TOC select
> > > > event. You can remove them.
> > > 
> > > I think that these values would be useful for some reasons:
> > > 
> > > 1. To seek to appropriate time in TOC select event handling. Otherwise how can
> > > I get time for particular UID?
> > 
> > Well, you would put the UID into the TOC select event and the element would
> > then handle everything needed, e.g. the conversion to time or something else.
> > For a playlist element there wouldn't be any times for example.
> 
> If I remove start/stop/format values, then element will have to store time data
> (start/stop if any) related to every UID somewhere in internal structure. And
> after receiving TOC select event element will look in that internal structure
> (and not in the TOC) to find positions for particalur UID. Is that right?

Well, they can always store things in the GstStructure. Maybe with fields in a element-specific namespace, e.g. "matroskademux::chapter-start", "matroskademux::chapter-uid" and things like that.

In contrast to this there would be fields like "duration" without a namespace which have a generic meaning.

(In reply to comment #54)
> Well, they can always store things in the GstStructure. Maybe with fields in a
> element-specific namespace, e.g. "matroskademux::chapter-start",
> "matroskademux::chapter-uid" and things like that.
> 
> In contrast to this there would be fields like "duration" without a namespace
> which have a generic meaning.

Or have structures in structures. That is you have a GstStructure as the value for the entry named e.g. "matroskademux". The element would just grab the specific substructure and access "chapter-start", "chapter-uid" directly.

(In reply to comment #55)
> (In reply to comment #54)
> > Well, they can always store things in the GstStructure. Maybe with fields in a
> > element-specific namespace, e.g. "matroskademux::chapter-start",
> > "matroskademux::chapter-uid" and things like that.
> > 
> > In contrast to this there would be fields like "duration" without a namespace
> > which have a generic meaning.
> 
> Or have structures in structures. That is you have a GstStructure as the value
> for the entry named e.g. "matroskademux". The element would just grab the
> specific substructure and access "chapter-start", "chapter-uid" directly.

I like that idea :) This way we can solve problem with storing start/stop positions. Also I can add helper function so that applications can extract those values from the structure.

Created attachment 167835 [details] [review]
Generic TOC support

Created attachment 167836 [details] [review]
Add TOC and TOC select events

Created attachment 167837 [details] [review]
Add TOC message

Created attachment 167838 [details] [review]
Add TOC query

Created attachment 167839 [details] [review]
Add new GstTocSetter interface

Created attachment 167840 [details] [review]
Check tests for the GstToc

Created attachment 167842 [details] [review]
Check tests for the GstTocSetter

Created attachment 167843 [details] [review]
Add GstToc and GstTocSetter sections in the docs

Here is a new patchset for GstToc support. What's new: add GstTocSetter interface (with unit tests), new _copy () methods in GstToc, remove start/stop and format values from the GstTocEntry, replace pad field in GstTocEntry with GList, add methods to set/get time positions of GstTocEntry (based on GstTocEntry.info field), update tests for the GstToc, update all docs to recent changes, other fixes and improvements.

I like the patches but your first patch depends on the tocsetter patch ;)

Now what would be nice are comments by other people...

(In reply to comment #66)
> I like the patches but your first patch depends on the tocsetter patch ;)
> 
> Now what would be nice are comments by other people...

I can fix that issue after somebody's else review (with some features if any would be need). Just backed to city and now I can work again :)

As the 1.0 release is coming closer I wanted to ask for the state of this feature request.

I had been hoping to improve the GME plugin (e.g. playing nintendo 8-bit, multi-song *.NSF files instead of just playing the first song).

With Alexanders current set of patches; would they be usefull in cases where there are many songs per file? The multi-song NSF-files could be treated as a playlist, but I think one would rather just have all the individial songs.

I think this is definitely more than a start. Can't we just add this to 0.10 to get some experience and ev. fix api warts in the 0.11 branch?

Having a docs/design/part-toc.txt would be nice to get a one pager overview how applications would use it and how elements would use it.

Yes, that would be a good plan. I was mainly waiting for someone else to review the general idea and API before doing that and you seem to be the first one who took a closer look :) Feel free to push it

Hi guys!
Nice to see that the work doesn't go to trash :)
I can adjust last patches in accordance with the current GStreamer state and write an API overview Stefan mentioned, but I need a bit of time to restore in my mind the things related to TOC work I've done before.

Alexander, both of that'd be great!

> Feel free to push it

I would still like to have a look at it before it's being pushed, but if someone checks the API and double-checks it against the requirements identified in the previous comments, that would be a big help already.

Created attachment 209758 [details] [review]
Add generic table of contents (TOC) support

Created attachment 209759 [details] [review]
Add TOC and TOC select events

Created attachment 209760 [details] [review]
Add TOC message

Created attachment 209761 [details] [review]
Add TOC query

Created attachment 209762 [details] [review]
Add GstTocSetter interface

Created attachment 209763 [details] [review]
Check tests for the GstToc

Created attachment 209764 [details] [review]
Check tests for the GstTocSetter

Created attachment 209765 [details] [review]
Add GstToc and GstTocSetter sections into the docs

I've updated the patches, a small decription of TOC usage for developers will be in couple of days

Alexander, did you used the GStreamer wiki for the project planning when you did the GSoC. Also wonder if you did any research on which elements could implement it (e.g. avi/wav -> riff-cue chunks, matroska/webm -> ?, ogg -> ?, mp4/mov ->?)

Basically if we agree on the API, we could try to get a GSoC student for this year to implement it in plugins + gst-discoverer/gst-lauch (to have a way to see them).

(In reply to comment #84)
> Alexander, did you used the GStreamer wiki for the project planning when you
> did the GSoC.

No. To be honest, at the GSoC start I hadn't thought that I would be hacking GStreamer. My project (Totem chapters plugin) was more Totem-related, but as I have done it to the middle of the summer, my mentor Bastien Nocera suggested me to work further on GStreamer chapters support. So for me it was a bit spontaneously. I have read fully Application Development Manual and Plugin Writer's Guide to understand GStreamer architecture, and used other elements as examples for coding features. I have been also familiar with GObject at that moment, so maybe it helped me to start working with GStreamer codebase rather quickly.

> Also wonder if you did any research on which elements could
> implement it (e.g. avi/wav -> riff-cue chunks, matroska/webm -> ?, ogg -> ?,
> mp4/mov ->?)

As part of my GSoC I have been working on Matroska chapters implementation using GstToc (see bug 481070). So at least one element has already patches for current GstToc API. If we agree on GstToc API I can also make those patches into the shape. As for other formats, we told with Sebastian about DVD angles and CUE tracks if I remember correctly.

> Basically if we agree on the API, we could try to get a GSoC student for this
> year to implement it in plugins + gst-discoverer/gst-lauch (to have a way to
> see them).

That's would be really nice. If you need any help regarding GstToc for GSoC (maybe some explanations or other questions) - just let me know. Also students can use patches for Matroska element as an example of GstToc implementation.
Moreover, if we agree on GstToc API, there would be a great opportunity to improve Totem's built-in chapters plugin so it can handle at least Matroska chapters.

(In reply to comment #85)
> (In reply to comment #84)
> > Basically if we agree on the API, we could try to get a GSoC student for this
> > year to implement it in plugins + gst-discoverer/gst-lauch (to have a way to
> > see them).
> 
> That's would be really nice. If you need any help regarding GstToc for GSoC
> (maybe some explanations or other questions) - just let me know. Also students
> can use patches for Matroska element as an example of GstToc implementation.
> Moreover, if we agree on GstToc API, there would be a great opportunity to
> improve Totem's built-in chapters plugin so it can handle at least Matroska
> chapters.

I started a wiki-page:
http://gstreamer.freedesktop.org/wiki/ChapterSupport
We can add details as we go and find them.

Review of attachment 209758 [details] [review]:

::: gst/gsttoc.c
@@ +39,3 @@
+ * lists. You should use GST_TOC_ENTRY_TYPE_CHAPTER for generic TOC entry and
+ * GST_TOC_ENTRY_TYPE_EDITION for the entries which are considered to be alternatives
+ * (like DVD angles, Matroska editions and so on).

Is the hierarchy at most two levels deep?
GstToc
  GstTocEntry (in GstToc.entries)
    GstTocEntry (in GstToc.subentries) 
    GstTocEntry (in GstToc.subentries) 
    ...
  GstTocEntry (in GstToc.entries)
    GstTocEntry (in GstToc.subentries)
    ...
  ...


Or is it arbitrary, so that GstTocEntries can have a list of GstTocEntries again?

@@ +55,3 @@
+ * and gst_event_parse_toc_select() to parse received TOC select event. The same rule for
+ * the #GstMessage: gst_message_new_toc() to create new TOC #GstMessage, and
+ * gst_message_parse_toc() to parse received TOC message. Also you can create a new TOC query

For the toc parsing I am not sure how applications would iterate over the content. Could point to the totem chapters plugin sources in a reply on this bug?

@@ +482,3 @@
+GstToc *
+_gst_toc_from_structure (const GstStructure * toc)
+{

please add a comment for where this is needed.

@@ +647,3 @@
+GstStructure *
+_gst_toc_to_structure (const GstToc * toc)
+{

please add a comment for where this is needed.

(In reply to comment #87)
> Review of attachment 209758 [details] [review]:
> 
> ::: gst/gsttoc.c
> @@ +39,3 @@
> + * lists. You should use GST_TOC_ENTRY_TYPE_CHAPTER for generic TOC entry and
> + * GST_TOC_ENTRY_TYPE_EDITION for the entries which are considered to be
> alternatives
> + * (like DVD angles, Matroska editions and so on).
> 
> Is the hierarchy at most two levels deep?
> GstToc
>   GstTocEntry (in GstToc.entries)
>     GstTocEntry (in GstToc.subentries) 
>     GstTocEntry (in GstToc.subentries) 
>     ...
>   GstTocEntry (in GstToc.entries)
>     GstTocEntry (in GstToc.subentries)
>     ...
>   ...
> 
> 
> Or is it arbitrary, so that GstTocEntries can have a list of GstTocEntries
> again?

The hierarchy is arbitrary, so GstTocEntry has GList field named "subentries" which contains another GstTocEntries and so on. The only limitation is that chapters and editions can't be mixed at the same hierarchy level. In that case TOC supposed to be broken. As I remember, we have disscussed this moment with Sebastian and decided that mixed chapters and angles on the same level wouldn't have much meaning.

> @@ +55,3 @@
> + * and gst_event_parse_toc_select() to parse received TOC select event. The
> same rule for
> + * the #GstMessage: gst_message_new_toc() to create new TOC #GstMessage, and
> + * gst_message_parse_toc() to parse received TOC message. Also you can create
> a new TOC query
> 
> For the toc parsing I am not sure how applications would iterate over the
> content. Could point to the totem chapters plugin sources in a reply on this
> bug?

Totem chapters plugin doesn't support TOC. So there is no application which uses it at all. Well, the iteration is quite easy: just go through subentries recursively. Functions you've mentioned below do exactly the same thing. I can write an example of such usage in developer paper which is coming next days.

> @@ +482,3 @@
> +GstToc *
> +_gst_toc_from_structure (const GstStructure * toc)
> +{
> 
> please add a comment for where this is needed.
> 
> @@ +647,3 @@
> +GstStructure *
> +_gst_toc_to_structure (const GstToc * toc)
> +{
> 
> please add a comment for where this is needed.

These functions are used to parse TOC messages, events and queries. It seems that there is no need to make them public. Do you want to add comments about these functions right into the source code?

(In reply to comment #88)
> (In reply to comment #87)
> > Review of attachment 209758 [details] [review] [details]:
> > 
> > ::: gst/gsttoc.c
> > @@ +39,3 @@
> > + * lists. You should use GST_TOC_ENTRY_TYPE_CHAPTER for generic TOC entry and
> > + * GST_TOC_ENTRY_TYPE_EDITION for the entries which are considered to be
> > alternatives
> > + * (like DVD angles, Matroska editions and so on).
> > 
> > Is the hierarchy at most two levels deep?
> > GstToc
> >   GstTocEntry (in GstToc.entries)
> >     GstTocEntry (in GstToc.subentries) 
> >     GstTocEntry (in GstToc.subentries) 
> >     ...
> >   GstTocEntry (in GstToc.entries)
> >     GstTocEntry (in GstToc.subentries)
> >     ...
> >   ...
> > 
> > 
> > Or is it arbitrary, so that GstTocEntries can have a list of GstTocEntries
> > again?
> 
> The hierarchy is arbitrary, so GstTocEntry has GList field named "subentries"
> which contains another GstTocEntries and so on.

Oh right. Thats cool then.

> The only limitation is that
> chapters and editions can't be mixed at the same hierarchy level. In that case
> TOC supposed to be broken. As I remember, we have disscussed this moment with
> Sebastian and decided that mixed chapters and angles on the same level wouldn't
> have much meaning.

I see no problem with that. Makes sense.
> 
> > @@ +55,3 @@
> > + * and gst_event_parse_toc_select() to parse received TOC select event. The
> > same rule for
> > + * the #GstMessage: gst_message_new_toc() to create new TOC #GstMessage, and
> > + * gst_message_parse_toc() to parse received TOC message. Also you can create
> > a new TOC query
> > 
> > For the toc parsing I am not sure how applications would iterate over the
> > content. Could point to the totem chapters plugin sources in a reply on this
> > bug?
> 
> Totem chapters plugin doesn't support TOC. So there is no application which
> uses it at all. Well, the iteration is quite easy: just go through subentries
> recursively. Functions you've mentioned below do exactly the same thing. I can
> write an example of such usage in developer paper which is coming next days.

I guess I was looking for functions like gst_toc_foreach(toc, foreach_func, user_data), we can add those later if want though. This would hide that the implementation uses a GList.

> 
> > @@ +482,3 @@
> > +GstToc *
> > +_gst_toc_from_structure (const GstStructure * toc)
> > +{
> > 
> > please add a comment for where this is needed.
> > 
> > @@ +647,3 @@
> > +GstStructure *
> > +_gst_toc_to_structure (const GstToc * toc)
> > +{
> > 
> > please add a comment for where this is needed.
> 
> These functions are used to parse TOC messages, events and queries. It seems
> that there is no need to make them public. Do you want to add comments about
> these functions right into the source code?

Yep, keep them as is, I was mainly thinking of adding a comment line "These functions are used to parse TOC messages, events and queries."

Created attachment 210811 [details] [review]
Overview of GstToc usage

Here is a part-toc.txt with overview of GstToc usage which Stefan was mentioned.

Created attachment 210813 [details] [review]
Add generic table of contents (TOC) support

Add comments for GstToc functions prototyped in gst/gst_private.h

(In reply to comment #5)
> Some random thoughts:
> 
>  - why have a TOC query vs. posting a TOC message on the bus?
>    (query would take same path as seek event; but it's inconsistent
>    with how we announce tags; if it was a query, would we also have
>    a TOC-changed message then?)

Thats indeed different that how we do tags. Imho it should work that elements just wait for the toc event and parse it.

> 
>  - we'd need to allow for tree-like structures to be flexible, right?
>    Is this easy-enough to do in a way that doesn't make it extremely
>    painful to handle in apps? (we could predefine fixed hierarchy
>    schemes, like title:chapter or disc:tracks with a bunch of utility
>    functions for each)

We can add convenience api as we get a bit more experience when adding toc support to other elements.

> 
>  - do we need a 'TOC' at all? Why not just put it into the normal tags,
>    possibly with utility functions for easier parsing? (see e.g. the
>    rather messy and basic, and also commented out, GST_TAG_CDDA_TRACK_TAGS
>    stuff in cddabasesrc)

I see benefit from having it separate from tags. Taglists are list and tocs are by nature hierarchical.