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 695728 - x264enc plugin has a divide by zero error when framerate is "0/1"
x264enc plugin has a divide by zero error when framerate is "0/1"
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
1.0.1
Other Linux
: Normal major
: 1.0.7
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-12 20:47 UTC by tcdgreenwood
Modified: 2013-04-26 09:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for the issue (2.31 KB, patch)
2013-03-12 20:47 UTC, tcdgreenwood
needs-work Details | Review
Patch for issue reworked to remove C++ style comments (2.21 KB, patch)
2013-03-14 13:46 UTC, tcdgreenwood
committed Details | Review

Description tcdgreenwood 2013-03-12 20:47:47 UTC
Created attachment 238739 [details] [review]
Patch for the issue

Doing the following on Ubuntu 12.10 fails with divide by zero error.  Not sure where the error is but have a patch that seems to fix.  x264enc works great in a number of other scenarios.  I did have a similar issue on 1.0.5 using Mac but other updates seem to fix this so I'm not 100% sure where it fails at the moment.

The patch works by not setting the framerate on the x264 encoder and using VFR (as the framerate is unknown) I don't have a clock rate so I put a TODO in the patch for that as it should be used to set the timebase (though I'm not sure what x264 uses the timebase for) - I'm using RTP so I was tempted to hard code the timebase to the clockrate of 90000.  Also stopped setting the keyint_max based on the framerate as it just meant it would end up being 0.  If you think my patch is horrible I'd like to know why as I am keen to understand gstreamer better and am just starting on this journey.

One of my work colleagues also had this issue and worked around it by patching the rtpvp8depay element to set the framerate to "15/1".  But that wasn't just to prove it was the cause and didn't seem right as the framerate will be unknown in our final pipeline as we'll use RTP and SDP doesn't usually include framerate in our experience.

BTW I understand the pipeline is stupid, but it was the simplest thing I found reproduced this issue.

tgreenwood@tgreenwood-gstreamer:~$ gst-launch-1.0 -v videotestsrc ! vp8enc ! rtpvp8pay ! rtpvp8depay ! vp8dec ! x264enc ! rtph264pay ! udpsink name=sender_rtp_out port=5100 host=127.0.0.1
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
/GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0.GstPad:src: caps = video/x-raw, format=(string)I420, width=(int)320, height=(int)240, framerate=(fraction)30/1
/GstPipeline:pipeline0/GstVP8Enc:vp8enc0.GstPad:sink: caps = video/x-raw, format=(string)I420, width=(int)320, height=(int)240, framerate=(fraction)30/1
Redistribute latency...
/GstPipeline:pipeline0/GstVP8Enc:vp8enc0.GstPad:src: caps = video/x-vp8, profile=(string)0, streamheader=(buffer)< 4f56503830010100014000f00000010000010000001e00000001 >, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, framerate=(fraction)30/1
/GstPipeline:pipeline0/GstRtpVP8Pay:rtpvp8pay0.GstPad:src: caps = application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, payload=(int)96, ssrc=(uint)1423001955, timestamp-offset=(uint)80710173, seqnum-offset=(uint)36675
/GstPipeline:pipeline0/GstRtpVP8Depay:rtpvp8depay0.GstPad:src: caps = video/x-vp8, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstVP8Dec:vp8dec0.GstPad:sink: caps = video/x-vp8, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstRtpVP8Depay:rtpvp8depay0.GstPad:sink: caps = application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, payload=(int)96, ssrc=(uint)1423001955, timestamp-offset=(uint)80710173, seqnum-offset=(uint)36675
/GstPipeline:pipeline0/GstRtpVP8Pay:rtpvp8pay0.GstPad:sink: caps = video/x-vp8, profile=(string)0, streamheader=(buffer)< 4f56503830010100014000f00000010000010000001e00000001 >, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, framerate=(fraction)30/1
/GstPipeline:pipeline0/GstVP8Enc:vp8enc0.GstPad:src: caps = video/x-vp8, profile=(string)0, streamheader=(buffer)< 4f56503830010100014000f00000010000010000001e00000001 >, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, framerate=(fraction)30/1
/GstPipeline:pipeline0/GstRtpVP8Pay:rtpvp8pay0.GstPad:src: caps = application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, payload=(int)96, ssrc=(uint)1423001955, timestamp-offset=(uint)80710173, seqnum-offset=(uint)36675
/GstPipeline:pipeline0/GstRtpVP8Depay:rtpvp8depay0.GstPad:src: caps = video/x-vp8, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstVP8Dec:vp8dec0.GstPad:sink: caps = video/x-vp8, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstRtpVP8Depay:rtpvp8depay0.GstPad:sink: caps = application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, payload=(int)96, ssrc=(uint)1423001955, timestamp-offset=(uint)80710173, seqnum-offset=(uint)36675
/GstPipeline:pipeline0/GstRtpVP8Pay:rtpvp8pay0.GstPad:sink: caps = video/x-vp8, profile=(string)0, streamheader=(buffer)< 4f56503830010100014000f00000010000010000001e00000001 >, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, framerate=(fraction)30/1
/GstPipeline:pipeline0/GstRtpVP8Pay:rtpvp8pay0: timestamp = 80710173
/GstPipeline:pipeline0/GstRtpVP8Pay:rtpvp8pay0: seqnum = 36675
/GstPipeline:pipeline0/GstVP8Dec:vp8dec0.GstPad:src: caps = video/x-raw, format=(string)I420, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, colorimetry=(string)bt601, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstX264Enc:x264enc0.GstPad:sink: caps = video/x-raw, format=(string)I420, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, colorimetry=(string)bt601, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstVP8Dec:vp8dec0.GstPad:src: caps = video/x-raw, format=(string)I420, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, colorimetry=(string)bt601, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstX264Enc:x264enc0.GstPad:sink: caps = video/x-raw, format=(string)I420, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, colorimetry=(string)bt601, framerate=(fraction)0/1
Floating point exception (core dumped)

Hope this bug goes better than my last posting which was just horrible!! Thanks for your help.
Comment 1 Sebastian Dröge (slomo) 2013-03-13 08:24:10 UTC
Review of attachment 238739 [details] [review]:

::: ext/x264/gstx264enc.c
@@ +1055,3 @@
+  if (info->fps_d == 0 || info->fps_n == 0) {
+    // No FPS so must use VFR
+    // This raises latency apparently see http://mewiki.project357.com/wiki/X264_Encoding_Suggestions

Please use C-style comments, i.e. /* blablabla */

@@ +1058,3 @@
+    encoder->x264param.b_vfr_input = TRUE;
+    // TODO set timebase from clock rate
+    // Clock rate seems to not be sent through however

I don't think there even is a useful way of getting a clock rate here... and if it isn't used by x264 it doesn't matter anyway, right?
Comment 2 tcdgreenwood 2013-03-14 13:09:31 UTC
Thanks for the comments - if you are happy that we don't need the timebase then OK, but when using RTP I thought that I should really pass the clockrate through if possible as that is what I thought was correct.  I just couldn't see how to make the clockrate accessible as it disappears earlier on in the pipeline.  You are right that the codec seems to run fine either way.

I'll rework the patch if that helps.  I am way too used to Java/C++/ObjectiveC at the moment.
Comment 3 tcdgreenwood 2013-03-14 13:46:45 UTC
Created attachment 238873 [details] [review]
Patch for issue reworked to remove C++ style comments

I've reworked the comments in the patch and checked two pipelines:
gst-launch-1.0 videotestsrc ! 'video/x-raw,height=480,width=640,framerate=15/1' ! x264enc ! avdec_h264 ! videoconvert ! osxvideosink
and:
gst-launch-1.0 videotestsrc ! 'video/x-raw,height=640,width=480,framerate=15/1' ! vp8enc ! rtpvp8pay ! rtpvp8depay ! vp8dec ! queue ! x264enc ! avdec_h264 ! videoconvert ! osxvideosink
It seems to be working fine.  Please let me know if there is anything else I can do to help or if the patch needs any more work.  Thank you for helping me.
Comment 4 Sebastian Dröge (slomo) 2013-03-25 09:09:26 UTC
commit 2e38f24b56d906c25af5b270079aa9e822029c08
Author: Tom Greenwood <tgreenwood@Toms-MacBook-Pro.local>
Date:   Wed Mar 6 13:28:37 2013 +0000

    x264enc: Fix for 0/1 framerate - now uses VFR in this case
    
    Previously did a division by zero.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=695728