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 546328 - fractions with G_MAXINT cause failure for fixation
fractions with G_MAXINT cause failure for fixation
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal critical
: 0.10.21
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-08-04 22:00 UTC by Olivier Crête
Modified: 2008-08-05 15:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Find nearest fraction using doubles instead of fractions (2.18 KB, patch)
2008-08-04 22:19 UTC, Olivier Crête
committed Details | Review

Description Olivier Crête 2008-08-04 22:00:50 UTC
If we try to fixate a list { 15/2, 5/1 } around G_MAXINT/1 (like v4l2src tries to do with one of my webcams). It fails because gst_structure_fixate_field_nearest_fraction() does (G_MAXINT/1 - 15/2) and that fails. That prevents my otherwise perfectly fine Logitech quickcam Fusion from working with telepathy stream-engine.. 

Fixing that mean breaking the API in some way..We can either make the max num/demum to be sqrt(G_MAXINT)~ 46k .. Or we could store the num/demun into a 64bit...
Comment 1 Olivier Crête 2008-08-04 22:19:02 UTC
Created attachment 115860 [details] [review]
Find nearest fraction using doubles instead of fractions

I though about the problem a bit more and the solution became clear, the result does not have to be perfectly accurate to find which is the nearest, an approximation is OK, so we can use a double to do the maths, we can get there without having to worry about integer overflows.
Comment 2 David Schleef 2008-08-05 00:31:34 UTC
It's silly that v4l2src is using MAXINT as the preferred frame rate.  Something like 30 would be more appropriate.

The patch is probably a good idea anyway.
Comment 3 Olivier Crête 2008-08-05 14:22:05 UTC
I agree that maxint there is silly...  although if people start using it to get HD content, maybe 60 or 120 is a better idea. But maxint is used in other places where it makes sense..
Comment 4 Wim Taymans 2008-08-05 15:05:10 UTC
Something close to 320 x 240 at 30 fps seems like a good default. v4l2src seems to want to negotiate to the highest possible video size with the highest possible framerate which usually results in something too big and too slow and too unusable.

Commited your patch after making a unit test for it that found a bug in the code that I then also fixed.

        Patch by: Olivier Crete <tester at tester dot ca>

        * gst/gststructure.c:
        (gst_structure_fixate_field_nearest_fraction):
        Avoid overflows in fixation code when dealing with MAXINT values, which
        v4l2src seems to do.
        Fixes #546328.

        * tests/check/gst/gststructure.c: (GST_START_TEST):
        Make a unit test to check the fix.