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 764769 - matroskamux: make timecodescale configurable
matroskamux: make timecodescale configurable
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-08 09:45 UTC by Nicola
Modified: 2016-04-11 07:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
make timecodescale configurable (3.24 KB, patch)
2016-04-08 09:45 UTC, Nicola
none Details | Review
updated patch (3.13 KB, patch)
2016-04-08 11:11 UTC, Nicola
committed Details | Review

Description Nicola 2016-04-08 09:45:11 UTC
Created attachment 325576 [details] [review]
make timecodescale configurable

In a very specialized app I produce buffers with timestamp equal to millisecond and different for microsecond/nanosecond, the default timecode scale used in matroskamux will produce blocks with the same timestamp in this case since it use millisecond precision

Tha attached patch make timecodescale configurable using a property.

You can test with this pipeline:

videotestsrc ! jpegenc ! matroskamux ! filesink ...

mkvinfo shows  1000000 as timecodescale 

if you now use this pipeline:

filesrc ! matroskademux ! fakesink

you'll see buffers with millisecond precision

setting timecodescale on the muxer you can change the precision and mkvinfo will show the value you set.

See also this for a more general use case

https://matroska.org/technical/specs/notes.html#TimecodeScale
Comment 1 Sebastian Dröge (slomo) 2016-04-08 10:05:32 UTC
Review of attachment 325576 [details] [review]:

Makes sense, yes

::: gst/matroska/matroska-mux.c
@@ +342,3 @@
+      g_param_spec_int64 ("timecodescale", "TimecodeScale used to calculate "
+          "the Raw Timecode of a Block",
+          "Using the default (1000000) you'll get block with "

"a block" or "blocks"?
Comment 2 Nicola 2016-04-08 10:09:14 UTC
blocks :-) do you want an updated patch?
Comment 3 Sebastian Dröge (slomo) 2016-04-08 10:20:32 UTC
If you can do that, that would be great :)
Comment 4 Tim-Philipp Müller 2016-04-08 10:26:02 UTC
Comment on attachment 325576 [details] [review]
make timecodescale configurable

>+  g_object_class_install_property (gobject_class, PROP_TIMECODESCALE,
>+      g_param_spec_int64 ("timecodescale", "TimecodeScale used to calculate "
>+          "the Raw Timecode of a Block",
>+          "Using the default (1000000) you'll get block with "
>+          "milliseconds precision, using 1 nanoseconds precision", 1,
>+          GST_SECOND, DEFAULT_TIMECODESCALE,
>+          G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));

I think this should be something like:

"timecodescale", "TimecodeScale", "TimecodeScale used to calculate the Raw Timecode of a Block".

Don't see why you need to describe the defaults here, and we may want to change the default in future (e.g. be adaptive based on the input framerate or such to minimise rounding errors).
Comment 5 Tim-Philipp Müller 2016-04-08 10:27:01 UTC
or even "Timecode Scale" for the second :)
Comment 6 Nicola 2016-04-08 11:11:17 UTC
Created attachment 325580 [details] [review]
updated patch

Thanks for the review! 

Regarding the adaptative time scale will be useful for audio only files but probably audio only muxing should be fixed first (see 754696)
Comment 7 Sebastian Dröge (slomo) 2016-04-11 07:17:45 UTC
commit cbdbfc89023a734034d4a746862d5af4c4ac8483
Author: Nicola Murino <nicola.murino@gmail.com>
Date:   Fri Apr 8 13:05:57 2016 +0200

    matroskamux: make timecodescale configurable
    
    In some use cases the default timecodescale will produce blocks with the same timestamp
    
    https://bugzilla.gnome.org/show_bug.cgi?id=764769