GNOME Bugzilla – Bug 740130
matroskamux: wrong duration on some files
Last modified: 2015-08-11 21:31:31 UTC
please take a look at the file here: http://195.250.34.59/temp/wrong_duration.mkv this file is without indexes if you try to add indexes with a pipeline like this: filesrc ! matroskademux ! matroskamux ! filesink you'll get a file with a duration header of about 6 minutes the buffer timestamp indicate that the file is 5 minutes, mkvmerge -o out.mkv wrong_duration.mkv produce a file with a 5 minutes duration header, this is correct maybe gstreamer matroskamux get confused from timestamp that occasionally go backwards, for example: 0:05:00.098000000 0:05:00.065000000
I think the problem is here: http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/matroska/matroska-mux.c#n3324 the input file has 9025 frames and each frame has a duration setted to 0:00:00.040000000 but the real duration of each buffer is less than 40 milliseconds. I guess the problem arise since some gstreamer plugin assume 25 fps somewhere, for example matroskamux, and write an invalid header for file such this with 30 fps, suggestions?
a quick (workaround?) is not set duration at all in matroskamux, probably is the best solution here: better a missing information than a wrong one
Created attachment 290744 [details] file that show the initial problem The initial problem can be see with the attached file: gst-launch-1.0 -v filesrc location=/tmp/test.gdp ! gdpdepay ! fakesink show: /GstPipeline:pipeline0/GstFakeSink:fakesink0.GstPad:sink: caps = "video/x-h264\,\ stream-format\=\(string\)avc\,\ alignment\=\(string\)au\,\ codec_data\=\(buffer\)014d401effe1000f674d401e9a6604012d80b50501050201000468ee3c80\,\ width\=\(int\)512\,\ height\=\(int\)288\,\ framerate\=\(fraction\)0/1\,\ parsed\=\(boolean\)true\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ level\=\(string\)3\,\ profile\=\(string\)main" so framerate is 0/1 now: gst-launch-1.0 -v filesrc location=/tmp/test.gdp ! gdpdepay ! matroskamux ! filesink location=/tmp/test.mkv and finally: gst-launch-1.0 -v filesrc location=/tmp/test.mkv ! matroskademux streamable=true ! fakesink show: /GstPipeline:pipeline0/GstFakeSink:fakesink0.GstPad:sink: caps = "video/x-h264\,\ level\=\(string\)3\,\ profile\=\(string\)main\,\ codec_data\=\(buffer\)014d401effe1000f674d401e9a6604012d80b50501050201000468ee3c80\,\ stream-format\=\(string\)avc\,\ alignment\=\(string\)au\,\ width\=\(int\)512\,\ height\=\(int\)288\,\ framerate\=\(fraction\)25/1" so the framerate is now 25/1 and this cause a wrong 40 milliseconds duration and so a wrong duration in matroska header if you remux to add indexes
I did some more analysis, the problem is generated here: http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/matroska/matroska-demux.c#n5079 so the demuxer hardcode 25 fps: /* sort of a hack to get most codecs to support, * even if the default_duration is missing */ this cause problem if the stream has more than 25 fps and no duration is setted. Using something like this gst_structure_set (structure, "framerate", GST_TYPE_FRACTION, 0, 1, NULL); seems better and it works for my use case, what do you think about? thanks
Created attachment 290924 [details] [review] possible fix this patch set framerate as 0/1 when duration is not known, this should be the right fix, if you want I can preserve the hack and set framerate as 30/1 or 60/1 this will cause a wrong frame duration, however there is already some logic inside the muxer to handle with that (in the case the file is remuxed) http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/matroska/matroska-mux.c#n2270
can this patch be reviewed for 1.6 cycle? thanks
This is probably a leftover from the times when unknown framerates were a real problem. commit c466ff474859cce91e9dd9b906c48f5fe0c4b164 Author: Nicola Murino <nicola.murino@gmail.com> Date: Tue Nov 18 16:52:40 2014 +0100 matroskademux: set framerate 0/1 when duration is not known https://bugzilla.gnome.org/show_bug.cgi?id=740130
That commit broke this test: https://ci.gstreamer.net/job/GStreamer-master-validate/122/testReport/validate.file.transcode/to_mp3_and_h264_in_mp4/vorbis_vp8_0_webm/ So basically you can reproduce with: gst-validate-launcher --sync -t validate.file.transcode.to_mp3_and_h264_in_mp4.vorbis_vp8_0_webm Reopening the bug and putting it as BLOCKER as it is a regression.
Setting #734424 blocking that bug because the regression introduced in that commit is actually a result of that bug (which we were not noticing previously because we were exposing a fake framerate).
So this here is fixed once videorate is fixed?
We also need another patch in the discoverer so that it plug videorate. I pushed it here: http://phabricator.freedesktop.org/D37 I am not 100% we want to go this way, so please let me know. (let me know if you prefer I attach it here)
> (let me know if you prefer I attach it here) Yes, please don't put patches from the main modules into phabricator until we've migrated, and attach it here.
So what exactly is it that makes the validate test fail?
Created attachment 303363 [details] [review] discoverer: let videorate compute a framerate when none provided Summary: And make sure that the updated values are properly set on the main GstDiscovererStreamInfo caps value (and not only its structure values)
Comment on attachment 303363 [details] [review] discoverer: let videorate compute a framerate when none provided I don't think this is the right approach. If we want to make up framerates, it'd probably be better to let the demuxer make them up, because it has more and better information. It's also not clear to me at all that (a) making up / guesstimating a framerate is better than signalling that there is no fixed framerate when there is none, and (b) that this is actually a blocker
I do not really care tbh, I guess we can just WONTFIX that bug as current behaviour is not that bad (signalling that there is no fixed framerate).
Ok, great, thanks. Just to make sure I understood the situation correctly, here is my understanding: - before matroskademux used to just say framerate=25/1 when we didn't know - that was changed to say framerate=0/1 (variable framerate) when we didn't know - validate tests broke because they were expecting 25/1 - now it might be desirable to investigate if we can advertise a better/proper framerate instead of 0/1, but 0/1 is not worse than a completely made up 25/1, so we may just as well update the validate tests and leave the task to figure out something better for some other time.
We need to update the media file then. https://ci.gstreamer.net/job/GStreamer-master-validate/lastCompletedBuild/testReport/validate.http/media_check/vorbis_vp8_1_webm/
commit bcb2afbf221abbae3a479cb55185a28fb369492d Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk> Date: Tue Aug 11 09:13:15 2015 -0400 media: MKV framerate use to be a random value It's now properly set to 0/1, letting GStreamer entirely rely on timestamps as it should.
Thanks Nicolas, we need a gst-validate-integration-testsuites product/project in the bug tracker actually :)