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 695709 - vp8enc plugin has invalid parameter error when framerate is "0/1"
vp8enc plugin has invalid parameter error when framerate is "0/1"
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.0.5
Other All
: Normal major
: 1.0.7
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-12 15:26 UTC by tcdgreenwood
Modified: 2015-05-15 14:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Output from the second script that re-encodes the stream (3.13 KB, text/plain)
2013-03-12 18:28 UTC, tcdgreenwood
  Details
Patch for the issue (5.02 KB, patch)
2013-03-12 18:33 UTC, tcdgreenwood
needs-work Details | Review
Patch for the issue with added comment as to why assume 90000 clockrate (5.26 KB, patch)
2013-03-14 14:01 UTC, tcdgreenwood
committed Details | Review
Patch for 0 framerate including timebase property (7.98 KB, patch)
2013-05-10 14:20 UTC, tcdgreenwood
none Details | Review
Patch to add property on master (5.59 KB, patch)
2013-05-24 18:00 UTC, tcdgreenwood
committed Details | Review

Description tcdgreenwood 2013-03-12 15:26:27 UTC
I've reproduced this issue on Mac and Linux (Ubuntu).
Comment 1 tcdgreenwood 2013-03-12 15:27:45 UTC
Sorry I accidentally posted this bug too early!

The bug is that
Comment 2 tcdgreenwood 2013-03-12 18:22:42 UTC
Sorry I accidentally posted this bug too early!

When having decoded an RTP stream the framerate is 0/1 and this causes a divide by zero error.  The use case is transcoding, but the script here just decodes and recodes VP8.  The scripts used were:

Receiver:
gst-launch-1.0 udpsrc name=receiver_rtp_in port=5100 caps = "application/x-rtp, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, width=(int)640, height=(int)480, payload=(int)96" ! rtpvp8depay  ! vp8dec ! queue ! videoconvert ! osxvideosink

Re-coder:
gst-launch-1.0 -v udpsrc name=receiver_rtp_in port=5000 caps = "application/x-rtp, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, width=(int)640, height=(int)480" ! rtpvp8depay ! queue ! vp8dec ! queue ! vp8enc ! rtpvp8pay ! udpsink name=sender_rtp_out port=5100 host=0:0:0:0:0:ffff:7f00:1 ts-offset=0

sendVP8.sh:
gst-launch-1.0 -v rtpbin name=rtpsender videotestsrc ! textoverlay text="Test Text" ! 'video/x-raw,width=640,height=480,framerate=15/1' ! vp8enc cpu-used=8 deadline=1 ! rtpvp8pay ! rtpsender.send_rtp_sink_0 rtpsender.send_rtp_src_0 ! udpsink port=5000 host=0:0:0:0:0:ffff:7f00:1 ts-offset=0 name=vrtpsink rtpsender.send_rtcp_src_0 ! udpsink port=5001 host=0:0:0:0:0:ffff:7f00:1 sync=false async=false name=vrtcpsink udpsrc name=receiver_rtcp_in port=5003 caps = "application/x-rtcp" ! rtpsender.recv_rtcp_sink_0

The udpsink elements use 0:0:0:0:0:ffff:7f00:1 as on Mac it seems that builds cannot use IPv4 addresses - this is 127.0.0.1.

The error I get is:
ERROR: from element /GstPipeline:pipeline0/GstVP8Enc:vp8enc0: Failed to initialize encoder

Will attach full output.
Comment 3 tcdgreenwood 2013-03-12 18:23:07 UTC
Sorry I accidentally posted this bug too early!

When having decoded an RTP stream the framerate is 0/1 and this causes a divide by zero error.  The use case is transcoding, but the script here just decodes and recodes VP8.  The scripts used were:

Receiver:
gst-launch-1.0 udpsrc name=receiver_rtp_in port=5100 caps = "application/x-rtp, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, width=(int)640, height=(int)480, payload=(int)96" ! rtpvp8depay  ! vp8dec ! queue ! videoconvert ! osxvideosink

Re-coder:
gst-launch-1.0 -v udpsrc name=receiver_rtp_in port=5000 caps = "application/x-rtp, clock-rate=(int)90000, encoding-name=(string)VP8-DRAFT-IETF-01, width=(int)640, height=(int)480" ! rtpvp8depay ! queue ! vp8dec ! queue ! vp8enc ! rtpvp8pay ! udpsink name=sender_rtp_out port=5100 host=0:0:0:0:0:ffff:7f00:1 ts-offset=0

sendVP8.sh:
gst-launch-1.0 -v rtpbin name=rtpsender videotestsrc ! textoverlay text="Test Text" ! 'video/x-raw,width=640,height=480,framerate=15/1' ! vp8enc cpu-used=8 deadline=1 ! rtpvp8pay ! rtpsender.send_rtp_sink_0 rtpsender.send_rtp_src_0 ! udpsink port=5000 host=0:0:0:0:0:ffff:7f00:1 ts-offset=0 name=vrtpsink rtpsender.send_rtcp_src_0 ! udpsink port=5001 host=0:0:0:0:0:ffff:7f00:1 sync=false async=false name=vrtcpsink udpsrc name=receiver_rtcp_in port=5003 caps = "application/x-rtcp" ! rtpsender.recv_rtcp_sink_0

The udpsink elements use 0:0:0:0:0:ffff:7f00:1 as on Mac it seems that builds cannot use IPv4 addresses - this is 127.0.0.1.

The error I get is:
ERROR: from element /GstPipeline:pipeline0/GstVP8Enc:vp8enc0: Failed to initialize encoder

Will attach full output.
Comment 4 tcdgreenwood 2013-03-12 18:28:55 UTC
Created attachment 238727 [details]
Output from the second script that re-encodes the stream

The error can be seen here:

ERROR: from element /GstPipeline:pipeline0/GstVP8Enc:vp8enc0: Failed to initialize encoder
Additional debug info:
gstvp8enc.c(1543): gst_vp8_enc_set_format (): /GstPipeline:pipeline0/GstVP8Enc:vp8enc0:
invalid parameter
Comment 5 tcdgreenwood 2013-03-12 18:33:41 UTC
Created attachment 238728 [details] [review]
Patch for the issue

This is a patch for the 1.0.5 release tag.  I couldn't vouch for the quality as this is one of the first changes I've made to gstreamer but I've tried to think through how it should work.  In particular I'm not sure about the timebase which I've got from the RTP payloading specification clockrate of 90000 which seems a reasonable default but might not be the best option.

Many apologies for messing up posting this bug so bad - I seem to have completely lost control of my keyboard.  If there is anything I can do to help with this fix please let me know.  BTW I've encountered a similar issue on x264enc, but I failed to reproduce now and have a patch for that also.
Comment 6 tcdgreenwood 2013-03-12 20:53:33 UTC
Single pipeline that reproduces the same issue (I hope this is easier for you to work with):

gst-launch-1.0 -v videotestsrc ! vp8enc ! rtpvp8pay ! rtpvp8depay ! vp8dec ! vp8enc ! rtpvp8pay ! udpsink name=sender_rtp_out port=5100

the rtpvp8depay causes the framerate to go from 30/1 to 0/1 which causes the issue in vp8enc.  Again if there is any way I can help let me know.
Comment 7 Sebastian Dröge (slomo) 2013-03-13 08:27:02 UTC
Review of attachment 238728 [details] [review]:

::: ext/vpx/gstvp8enc.c
@@ +1512,3 @@
+  if (GST_VIDEO_INFO_FPS_D (info) == 0 || GST_VIDEO_INFO_FPS_N (info) == 0) {
+    encoder->cfg.g_timebase.num = 1;
+    encoder->cfg.g_timebase.den = 90000;

Why use a timebase of 1/90000 here? Please add a comment at least :)
Comment 8 tcdgreenwood 2013-03-14 14:01:19 UTC
Created attachment 238875 [details] [review]
Patch for the issue with added comment as to why assume 90000 clockrate

I've added a comment including the link and section in the specification.  I hope this is good enough reason.  If there is any further work required please let me know.  Thanks for all your help.
Comment 9 Sebastian Dröge (slomo) 2013-03-25 09:06:10 UTC
commit 4d0542220e8bd928220b16774d9fbfe308760122
Author: Tom Greenwood <tgreenwood@Toms-MacBook-Pro.local>
Date:   Wed Mar 6 13:17:54 2013 +0000

    vp8enc: Fix for divide by zero when using 0/1 framerate
    
    https://bugzilla.gnome.org/show_bug.cgi?id=695709
Comment 10 Tim-Philipp Müller 2013-03-25 11:00:10 UTC
I don't really understand this reasoning:

    /* Zero framerate but still need to setup the timebase so we
     * presume this is RTP

The simplest case I can think of is

  {v4l2src,ximagesrc} ! videoconvert ! vp8enc ! foomux ! ...

- no RTP.

At the very least we should try and fallback to the fps from the "max-framerate" field IMHO if there is one. (On a side note v4l2src doesn't seem to do the right thing, I think it should do framerate=0/1 for webcams where we are not guaranteed to get a fixed framerate, and then set max-framerate to whatever was configured).
Comment 11 tcdgreenwood 2013-04-10 17:55:48 UTC
Tim - I'd be happy with max-framerate - though I don't know about it's defaults.  I looked around and couldn't find another pipeline that did this so I made a presumption.  I didn't think about max-framerate because I didn't realise it existed - probably just my own lack of understanding.  I was using RTP so that is what I thought about - having a timebase shorter than that of the framerate shouldn't matter though as the timebase should be the shortest possible time between frames.  Alternatively we might want to add a timebase property to allow this to be set to 1/90000 in RTP scenarios.
Comment 12 Sebastian Dröge (slomo) 2013-04-15 06:26:14 UTC
What should we do about this? Use max-framerate if existing and otherwise fall back to a timebase property? What's the exact effect of this btw, it's used in bitrate calculations?
Comment 13 tcdgreenwood 2013-04-15 11:00:35 UTC
I'm not sure how the codec uses this.  For RTP I understand that the timebase could be used regardless of the framerate.  Setting the value too high or too low can have problems from discussions I seen with others around libvpx.  RTP based systems I've seen the code for seem to either use 1/90000 or 1000/90000000 or base on framerate.  I really don't understand the purpose of setting to 1000/90000000 - I think that was from a copy of mediastreamer a colleague was working on.

The docs for libvpx say it should be set to the shortest possible time between frames, but I am pretty sure it doesn't really represent the framerate per se.

If you think max-framerate should be taken into account I'll rework to have a specific timebase value that can be set.  I'm happy to do the work creating the patch or test something that you guys want to propose.  I found too many build issues to test patches on master however.

If you want I can look into this further?
Comment 14 tcdgreenwood 2013-04-15 11:00:36 UTC
I'm not sure how the codec uses this.  For RTP I understand that the timebase could be used regardless of the framerate.  Setting the value too high or too low can have problems from discussions I seen with others around libvpx.  RTP based systems I've seen the code for seem to either use 1/90000 or 1000/90000000 or base on framerate.  I really don't understand the purpose of setting to 1000/90000000 - I think that was from a copy of mediastreamer a colleague was working on.

The docs for libvpx say it should be set to the shortest possible time between frames, but I am pretty sure it doesn't really represent the framerate per se.

If you think max-framerate should be taken into account I'll rework to have a specific timebase value that can be set.  I'm happy to do the work creating the patch or test something that you guys want to propose.  I found too many build issues to test patches on master however.

If you want I can look into this further?
Comment 15 Sebastian Dröge (slomo) 2013-04-15 11:33:34 UTC
That would be great, yes
Comment 16 tcdgreenwood 2013-04-24 22:40:24 UTC
I have just been checking how to get max-framerate from the caps.  It turns out that video-info.c gst_video_info_from_caps pushes the max-framerate into the GstVideoInfo if framerate is 0.  So using max-framerate is already supported.

My proposed change is therefore to reflect this in comments, add a property and leave the setting to 1/90000 as a fall back if nothing else is set to ensure we avoid all possible divide by 0 errors.
Comment 17 Sebastian Dröge (slomo) 2013-04-25 07:51:06 UTC
Sounds like a good plan
Comment 18 tcdgreenwood 2013-05-10 14:20:32 UTC
Created attachment 243786 [details] [review]
Patch for 0 framerate including timebase property

I have created a new patch with the timebase property.  Property defaults to 0 which makes the plugin use the framerate (or max-framerate if framerate isn't available).  If that's still 0 it uses 1/90000 as a fallback (which comes from the RTP VP8 spec and should work OK for almost every case as far as I can tell).  This should avoid any more divide by zero problems.

Sorry this has taken so long - too busy!
Comment 19 Sebastian Dröge (slomo) 2013-05-11 07:18:04 UTC
This should probably be a GstFraction property, otherwise looks good :)
Comment 20 tcdgreenwood 2013-05-14 14:45:35 UTC
I will update - I was looking for something like that, but couldn't see one.  When I looked at videorate it defined the framerate as an int so I presumed that fraction properties didn't exist.
Comment 21 tcdgreenwood 2013-05-24 18:00:22 UTC
Created attachment 245259 [details] [review]
Patch to add property on master

This is a patch against master that adds the timebase property as a fraction.  I hope that this is now OK!

Thanks for the help.
Comment 22 Sebastian Dröge (slomo) 2013-05-26 08:23:35 UTC
commit 5532989728c215e3f6585dbb589c5a3bd7207ad6
Author: Tom Greenwood <tcdgreenwood@hotmail.com>
Date:   Wed Mar 6 13:17:54 2013 +0000

    vp8enc: Add property to manually specify the timebase of the encoder
    
    https://bugzilla.gnome.org/show_bug.cgi?id=695709