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 759055 - baseparse: post tag list updates on percentage delta instead of fixed delta
baseparse: post tag list updates on percentage delta instead of fixed delta
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-05 09:52 UTC by Athanasios Oikonomou
Modified: 2015-12-08 09:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
baseparse: post tag list when avg bitrate changes at least 2% (2.33 KB, patch)
2015-12-05 10:02 UTC, Athanasios Oikonomou
none Details | Review
baseparse: post tag list when avg bitrate changes at least 2% (2.36 KB, patch)
2015-12-07 16:34 UTC, Athanasios Oikonomou
committed Details | Review

Description Athanasios Oikonomou 2015-12-05 09:52:41 UTC
Watching videos with variant bitrate is common to have delta more than 10 kbps, resulting in tag list spam.

Instead of relying on fixed 10 kpbs delta, it is better to calculale the difference in percentage and update tag list only when bitrate changes more than 2%.

Patch follows.
Comment 1 Athanasios Oikonomou 2015-12-05 10:02:11 UTC
Created attachment 316800 [details] [review]
baseparse: post tag list when avg bitrate changes at least 2%
Comment 2 Sebastian Dröge (slomo) 2015-12-07 11:36:10 UTC
Review of attachment 316800 [details] [review]:

::: libs/gst/base/gstbaseparse.c
@@ +1759,3 @@
+    if (parse->priv->post_avg_bitrate && parse->priv->avg_bitrate) {
+      gint diffprev = (gint) 100 * (ABSDIFF (parse->priv->avg_bitrate,
+          parse->priv->posted_avg_bitrate)) / parse->priv->avg_bitrate;

That's not going to work. This will always be 0, 100 or an integer multiple of 100.

You first have to multiply, than divide. For example by using gst_util_uint64_scale_int().
Comment 3 Athanasios Oikonomou 2015-12-07 14:09:40 UTC
Review of attachment 316800 [details] [review]:

::: libs/gst/base/gstbaseparse.c
@@ +1759,3 @@
+    if (parse->priv->post_avg_bitrate && parse->priv->avg_bitrate) {
+      gint diffprev = (gint) 100 * (ABSDIFF (parse->priv->avg_bitrate,
+          parse->priv->posted_avg_bitrate)) / parse->priv->avg_bitrate;

The evaluation order is left to right, so first it multiplies and then it divides already.

I'll change patch to use gst_util_uint64_scale_int().

guint64 diffprev = gst_util_uint64_scale_int(100,
   ABSDIFF(parse->priv->avg_bitrate, parse->priv->posted_avg_bitrate),
   parse->priv->avg_bitrate);
Comment 4 Athanasios Oikonomou 2015-12-07 16:34:03 UTC
Created attachment 316890 [details] [review]
baseparse: post tag list when avg bitrate changes at least 2%

Make use of gst_util_uint64_scale_int.
Comment 5 Sebastian Dröge (slomo) 2015-12-08 09:35:17 UTC
commit d10c488d63638e12490f5036dc50581d22592b5e
Author: Athanasios Oikonomou <athoik@gmail.com>
Date:   Mon Dec 7 18:20:35 2015 +0200

    baseparse: post tag list when avg bitrate changes at least 2%
    
    Watching videos with variant bitrate is common to have delta
    more than 10 kbps, resulting in tag list spam.
    
    Instead of relying on fixed 10 kpbs delta, it is better to
    calculale the difference in percentage and update tag list
    only when bitrate changes more than 2%.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759055