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 765660 - baseparse: use gst_util_uint64_scale to avoid overflow.
baseparse: use gst_util_uint64_scale to avoid overflow.
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-27 08:39 UTC by kevin
Modified: 2018-11-03 12:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to fix the issue. (1.10 KB, patch)
2016-04-27 08:40 UTC, kevin
needs-work Details | Review
baseparse: reset data_bytecount in gst_base_parse_reset (1.14 KB, patch)
2017-02-17 12:33 UTC, abhimanyu.v
none Details | Review

Description kevin 2016-04-27 08:39:09 UTC
use gst_util_uint64_scale_int will overflow as parse->priv->avg_bitrate
is unsigned int.
Comment 1 kevin 2016-04-27 08:40:22 UTC
Created attachment 326842 [details] [review]
Patch to fix the issue.
Comment 2 kevin 2016-04-27 08:42:38 UTC
Will have below log without the patch for large bitrate stream.

(gplay-1.0:2125): GStreamer-CRITICAL **: _gst_util_uint64_scale_int: assertion 'denom > 0' failed
Comment 3 Sebastian Dröge (slomo) 2016-04-27 08:44:48 UTC
You get diffs or avg bitrates higher than 2 billion bits per second but less than 4 billion? There might be other problematic places related to the bitrate, and if you already get into those dimensions it might make sense to convert it to a 64 bit integer, and/or store it as bytes per second (do we already?), etc.
Comment 4 Sebastian Dröge (slomo) 2016-04-28 10:45:27 UTC
Can you provide a stream where this happens btw?
Comment 5 kevin 2016-04-29 02:24:18 UTC
I found the root cause is the special stream report invalid fps. It report 90000fps. So the calculate bitrate is very large and cause overflow.

We can add one invalid fps check in our special demuxer to fix the issue.

Our demuxer can demux the special stream. tsdemux will report fail.
Comment 6 kevin 2016-04-29 02:30:03 UTC
How to share large video file to you? video file size is 65M.
Comment 7 Sebastian Dröge (slomo) 2016-04-29 06:35:13 UTC
Use something like dropbox. If tsdemux fails on it, we should make it work :)
Comment 8 kevin 2016-04-29 08:45:31 UTC
Please get the video file:

https://www.dropbox.com/s/4aoeexntsu77jul/HD_8M.mpg?dl=0
Comment 9 Sebastian Dröge (slomo) 2016-04-29 08:58:34 UTC
Ok, this fails because "mpegtsbase.c:1328:mpegts_base_scan:<tsdemux0> Couldn't find any PCR within the first 655360 bytes". Where is the PCR? :)


Can you open another bug against tsdemux for this specific file? Inside baseparse what should happen probably is to guard against overflows not only in this place but in general, I don't think your patch is complete here as it could still overflow (bitrate is still an int).
Comment 10 Tim-Philipp Müller 2016-11-11 18:38:31 UTC
What shall we do with this? mpegtsbase warning still happens.
Comment 11 abhimanyu.v 2017-02-17 12:33:07 UTC
Created attachment 346063 [details] [review]
baseparse: reset data_bytecount in gst_base_parse_reset

I have usecase where we have master on one unit and slave on another. Master encoder pcm data in flac and send over to slave. On slave we dont recreate pipeline but reset the flacparser when new stream-start is detected to fix it internal variable. In this scenario i have seen 
GStreamer-CRITICAL **: _gst_util_uint64_scale_int: assertion 'denom > 0' failed
when you change the track etc.

Proposed fix fixes the issue for me, it actually reset one of the variable which get used to calculate avg_buffer.
Comment 12 GStreamer system administrator 2018-11-03 12:34:29 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/170.