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 669502 - [baseparse] duration msg spam when upstream knows avg bitrate
[baseparse] duration msg spam when upstream knows avg bitrate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 0.10.36
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-06 19:00 UTC by Jonas Larsson
Modified: 2012-02-11 16:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Sample stream (94.45 KB, video/mp4)
2012-02-06 19:00 UTC, Jonas Larsson
  Details
Ugly and hacky solution to show my point (758 bytes, patch)
2012-02-06 19:05 UTC, Jonas Larsson
none Details | Review
baseparse: bitrate update mechanics should not deal with duration update (1.36 KB, patch)
2012-02-07 10:33 UTC, Mark Nauwelaerts
committed Details | Review

Description Jonas Larsson 2012-02-06 19:00:44 UTC
Created attachment 206927 [details]
Sample stream

* gets avg bitrate tag from upstream, sets priv->post_avg_bitrate = FALSE
* gst_base_parse_update_bitrates uses priv->posted_avg_bitrate to determine if avg bitrate should be posted
* Since priv->post_avg_bitrate is FALSE, priv->posted_avg_bitrate is never set in gst_base_parse_post_bitrates
* gst_base_parse_update_bitrates sets update_avg TRUE for every frame
* duration msg is posted on the bus for every frame
* when frames are small (eg aac) the bus spam is significant

Sample stream attached to show the problem:
gst-launch -v -m filesrc location=spam.mp4 ! qtdemux ! aacparse ! fakesink
A duration msg is posted for each aac frame.
Comment 1 Jonas Larsson 2012-02-06 19:05:55 UTC
Created attachment 206928 [details] [review]
Ugly and hacky solution to show my point
Comment 2 Mark Nauwelaerts 2012-02-07 10:33:05 UTC
Created attachment 206967 [details] [review]
baseparse: bitrate update mechanics should not deal with duration update

Previous patch does not look so hackish.

However, alternatively, this one removes the duration update stuff in _update_bitrates altogether since there is already such duration update message heuristic in _update_duration.  And AFAICS/AFAIK, if the (proposed removed) test in _update_bitrates would trigger, so should then also the one in _update_duration, which will then take care.
Comment 3 Mark Nauwelaerts 2012-02-10 13:50:32 UTC
No objections, so let's use the latter approach:

commit 954dd59fdd91d21cb64c9447e3a967a4d1b8ce10
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Tue Feb 7 11:28:41 2012 +0100

    baseparse: bitrate mechanics should not deal with duration update
    
    ... since that is already handled by _update_duration, or should not be done
    altogether if the duration is determined by non-estimated means.
    
    Fixes #669502.
Comment 4 Tim-Philipp Müller 2012-02-11 16:21:07 UTC
Not directly related to this patch, but IMHO baseparse shouldn't be sending duration/bitrate tags/messages at all if upstream knows the duration or knows bitrates.
Comment 5 Tim-Philipp Müller 2012-02-11 16:24:54 UTC
PS: would be good to mention the effect of the change in the commit message as well next time (i.e. fixes duration message spam on the bus)