After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 644890 - [mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly
[mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-03-16 08:48 UTC by Andreas Frisch
Modified: 2012-10-10 11:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch outline for adding indexing to mpegtsmux (7.94 KB, patch)
2011-03-16 08:55 UTC, Andreas Frisch
none Details | Review
[mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode (6.30 KB, patch)
2011-03-21 16:23 UTC, Andreas Frisch
none Details | Review
[mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode (7.03 KB, patch)
2011-03-25 19:16 UTC, Andreas Frisch
none Details | Review
updated [mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode (6.76 KB, patch)
2011-03-28 08:39 UTC, Andreas Frisch
none Details | Review
[mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode (6.76 KB, patch)
2011-03-29 10:44 UTC, Andreas Frisch
none Details | Review
table with entry point comparison between old and new patch version (156.05 KB, application/x-bzip)
2012-06-20 15:51 UTC, Andreas Frisch
  Details

Description Andreas Frisch 2011-03-16 08:48:19 UTC
for bluray authoring, a list of entry points is requires for the so-called clipinfo files. those files contain a MAP of SPN / PTS value pairs.
SPN stands for Source Packet Number and is the offset from beginning of m2ts stream file divided by M2TS-packet size (192 bytes) for every keyframe and its respective Presentation Time Stamp.
i've added indexing capabilities to mpegtsmux and implemented the necessary parsing to determine whether a frame is an i-frame and to extract its exact PTS value in m2ts-mode. payload only runs through this additional logic if a GstIndex was excplicitely set by an application utilizing mpegtsmux.

patch is to be attached
Comment 1 Andreas Frisch 2011-03-16 08:55:41 UTC
Created attachment 183499 [details] [review]
proposed patch outline for adding indexing to mpegtsmux
Comment 2 Andreas Frisch 2011-03-16 08:57:55 UTC
remark: my iframe detection is currently limited to H.264 streams but MPEG-2 video isn't specified for bluray anyways
Comment 3 Andreas Frisch 2011-03-21 16:23:15 UTC
Created attachment 183960 [details] [review]
[mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode
Comment 4 Andreas Frisch 2011-03-25 19:16:27 UTC
Created attachment 184205 [details] [review]
[mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode

last diff was missing headers
Comment 5 Jan Schmidt 2011-03-26 09:16:53 UTC
In general the patch looks OK. It probably needs updating after my changes in master today though.

Also: MPEG-2 video is too specified for Blu-ray. The earliest releases used it (my Kiss Kiss Bang Bang disc is MPEG-2 1080p)
Comment 6 Andreas Frisch 2011-03-28 08:39:31 UTC
Created attachment 184427 [details] [review]
updated [mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode

i've updated the patch to work with thaytan's latest commit http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=9a26173a57b05ecb497fe1fca0168b5cbd4c0167

the indexing will work with any type of video elementary stream now, because i don't parse the PES packets by hand anymore, instead i leave that job to as respective upstream parser element. i've only tested it with h264 atm but it should just as well work with mpeg 2.
Comment 7 Andreas Frisch 2011-03-29 10:44:18 UTC
Created attachment 184562 [details] [review]
[mpegtsmux] add indexing capabilities to generate a SPN/PTS map on the fly in m2ts-mode
Comment 8 Mark Nauwelaerts 2012-06-11 12:42:11 UTC
Am looking at integration this, and a question (wrt specs); is an entry needed for every packet belonging to a keyframe (access point), or an entry for the first packet belonging to such a keyframe (the latter would seem to be sufficient for performing a seek or so ?).
Comment 9 Jan Schmidt 2012-06-11 13:39:16 UTC
I'm not sure I understand your distinction. As I understand it, only the first packet that contains the first byte of the start code for a keyframe is considered an access point - not 'every packet belonging to a keyframe'

I don't know what the spec says, but I'd be surprised if it says anything other than you put one entry per half second or so, pointing at the first packet that contains a piece of the first keyframe for that half second, and GOPs should be structured such that every half second of video is self contained, like it was on DVD.
Comment 10 Andreas Frisch 2012-06-11 13:59:49 UTC
the first frame of every access point (or entry point) is what goes into the bluray clipinfo
since we don't touch the video elementary streams or their gop structure, we rely on what the broadcast contains but what i've observed in my experiments is, that bluray players can handle (play+trickmodes) regular dvb streams just fine without any need to re-encode them as long as the ep maps are correct.
Comment 11 Mark Nauwelaerts 2012-06-11 14:50:48 UTC
Well, the distinction is that the original patch added a GstIndex entry for
every packet of a keyframe (which then seems not necessary).

Following patch only adds one entry for a keyframe, along with some other
fixes/modifications:

commit 314480db5aa43af341bcc59ddd27eb347fb76872
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Mon Jun 11 16:44:02 2012 +0200

    mpegtsmux: add SPN/PTS indexing capabilities

    Based on patch by Andreas Frisch <fraxinas@opendreambox.org>

    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=644890
Comment 12 Andreas Frisch 2012-06-20 15:50:08 UTC
i'd like to provide a comparison between the output of the new behaviour from current git (left side) and my old original patch (right side)

my old patch was apparently considering too many frames to be keyframes, sometimes even two in a row

but the new one does very strange things, too
there are so-and-so many indexed frames with a -1 timestamp and then also, many increasing frames show up with the same pts over and over again
Comment 13 Andreas Frisch 2012-06-20 15:51:50 UTC
Created attachment 216849 [details]
table with entry point comparison between old and new patch version
Comment 14 Mark Nauwelaerts 2012-06-21 08:49:02 UTC
One the one hand, it is not clear exactly what the table represents.

On the other hand, from having a look at current code, and running some tests (e.g. with videotestsrc ! ffenc_mpeg4), I don't see what could be going wrong there.  It basically states, if a keyframe, add an entry with the count and timestamp.

So, ending up with wrong data is then only possible if the input data is bogus, i.e. wrong DELTA flags and/or timestamps.  As such, it appears to come down to what the input is here.  Also note that non-ascending input timestamps, e.g. due to b-frames, may not be handled too well, but that applies not only to the index.
Comment 15 Andreas Frisch 2012-06-21 09:21:37 UTC
the table is simply the output of the index spn count and the pts value

static void entry_added (GstIndex * index, GstIndexEntry * entry, App * app) {
...
  if (entry->id == 1 && GST_INDEX_NASSOCS (entry) == 2) {
    g_fprintf (app->f_epmap, "entrypoint: %" G_GINT64_FORMAT " ",
      GST_INDEX_ASSOC_VALUE (entry, 0));
    g_fprintf (app->f_epmap, "%" G_GINT64_FORMAT "\n", GST_INDEX_ASSOC_VALUE
      (entry, 1));
    fflush(app->f_epmap);

i have not investigated, how the pts value can become -1. i had of course used the same exact input .ts file (a dvb-s recording) for both maps

here's the code of the bdremux tool (which prints the ep-map)
http://git.opendreambox.org/?p=bdremux.git;a=blob;f=src/bdremux.c

i'm uploading the sample ts too
Comment 17 Mark Nauwelaerts 2012-06-22 08:55:27 UTC
It seems that the pipeline in question essentially has (o.a.) 
tsdemux ! h264parse ! tsmux 

It then also seems that h264parse is trying to be too clever and switches to pass-through since upstream caps are ok for downstream.  Unfortunately, there is no keyframe info from upstream so everything arriving at tsmux appears as keyframe.  This leads to a lot of entries (with duplicate pts as well), no pts -1 although (afaics) (also timestamps coming from tsdemux are not always ascending).

Forcing h264parse to really parse by e.g.
tsdemux ! h264parse ! video/x-h264,alignment=(string)au ! tsmux 
leads to decent index data (based on some extra added debug statements).
Comment 18 Jan Schmidt 2012-06-22 09:08:51 UTC
Should mpegtsmux specify parsed=true perhaps?
Comment 19 Mark Nauwelaerts 2012-06-22 09:14:49 UTC
That would not really make a difference at present unless tsdemux would also specify parsed=false.  If it did so, then don't think tsmux has to specify anything.
Comment 20 Andreas Frisch 2012-10-10 11:34:14 UTC
how can i port this to the 1.0 with GstIndex gone?
Comment 21 Tim-Philipp Müller 2012-10-10 11:50:22 UTC
I don't think you can (easily). Easiest way would probably be to post some kind of element message for the application.