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 703350 - qtdemux: Reports wrong framerate
qtdemux: Reports wrong framerate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.1.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-06-30 14:39 UTC by Matej Knopp
Modified: 2013-07-10 07:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.43 KB, patch)
2013-06-30 14:39 UTC, Matej Knopp
needs-work Details | Review
Patch (2.18 KB, patch)
2013-06-30 19:04 UTC, Matej Knopp
committed Details | Review
Correct argument order in gst_util_uint64_scale_int_round (841 bytes, patch)
2013-07-09 17:12 UTC, Matej Knopp
committed Details | Review

Description Matej Knopp 2013-06-30 14:39:47 UTC
Created attachment 248087 [details] [review]
Patch

Currently qtdemux takes one sample from first 20 samples and uses its duration to compute framerate. For some files this can give invalid results (one file I tried this with shows 35fps). 

Also the timescale that is usually used (2400) means that single sample duration can not be used to compute framerate of 24000/1001. Best case scenario currently is that the sample will have duration of 100, resulting in 24fps, but that's not precise. Some samples have duration 99 making the average framerate 23.98.

Patch uses average sample duration and scales the timescale appropriately (so that it gives proper framerate for 24000/1001).
Comment 1 Matej Knopp 2013-06-30 14:47:29 UTC
I meant 101 instead of 99 in previous comment of course.
Comment 2 Sebastian Dröge (slomo) 2013-06-30 17:13:36 UTC
Review of attachment 248087 [details] [review]:

::: gst/isomp4/qtdemux.c
@@ +5464,3 @@
+      /* we might need to scale the timescale to get precise framerate */
+      const int required_scale = log10 (10000);
+      int current_scale = log10 (stream->timescale);

You need to link with $(LIBM) for this, and also log10() is not available everywhere. Only log() is safe to use.

@@ +5474,3 @@
+        stream->fps_d =
+            floor ((double) (stream->duration * factor) /
+            (double) stream->n_samples + 0.5);

gst_util_uint64_scale_int_round() maybe?
Comment 3 Matej Knopp 2013-06-30 19:04:23 UTC
Created attachment 248098 [details] [review]
Patch

Added libm, uses log instead and gst_util_uint64_scale_int_round
Comment 4 Sebastian Dröge (slomo) 2013-07-01 10:54:06 UTC
commit 4053e1d6acc241d7690c7b1325d3b43775b83e48
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Sun Jun 30 21:01:20 2013 +0200

    qtdemux: compute framerate from average sample duration
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703350
Comment 5 Matej Knopp 2013-07-09 17:12:56 UTC
Created attachment 248754 [details] [review]
Correct argument order in gst_util_uint64_scale_int_round

This needs to be applied otherwise the call fails with long streams. I'm not sure how I missed it.
Comment 6 Sebastian Dröge (slomo) 2013-07-10 07:20:47 UTC
commit 7b69f427f11e9c936e624e34863f2956232af5b1
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Tue Jul 9 19:10:17 2013 +0200

    qtdemux: correct argument order in gst_util_uint64_scale_int_round
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703350