GNOME Bugzilla – Bug 678447
PATCH: optimize vp8 encoding parameters for realtime encoding
Last modified: 2014-04-01 08:17:16 UTC
Created attachment 216789 [details] [review] Patch: optimize encoding parameters for realtime encoding Hi, We (Fedora) have been receiving a lot of complaints about cheese performing poorly when recording videos, see: https://bugzilla.redhat.com/show_bug.cgi?id=572169 Partially, this is to be expected given that we rely on the CPU for real-time encoding which cannot always keep up, but we can do a lot better by tweaking the vp8 encoding parameters to be more suitable for realtime encoding, like the gnome-shell screencast recording code does. The attach patch (which is inspired by the gnome-shell code) does this. With this patch the framerate when encoding a 1920x1080 video on my "Intel(R) Core(TM)2 Duo CPU P9500 @ 2.53GHz" laptop increases from aprox. 1 frame every 2 seconds to aprox. 10 fps, iow from unusable to usable but not perfect. And with this patch that machine can record videos flawlessly at 1280x720.
Review of attachment 216789 [details] [review]: ::: cheese-3.4.2.orig/libcheese/cheese-camera.c @@ -456,1 +458,6 @@ - g_object_set (G_OBJECT (video_enc), "speed", 2, NULL); + + /* Since we do realtime encoding setup the encoder for speed without + sacrificing too much quality */ ... 3 more ... I do not think that Cheese should check the number of processors and calculate the number of threads to use, as the encoder is supposed to do this automatically if the ‘token-parts’ property is set to > 0: http://www.webmproject.org/tools/encoder-parameters/#6-multi-threaded-encode-and-decode
(In reply to comment #1) > I do not think that Cheese should check the number of processors and calculate > the number of threads to use, as the encoder is supposed to do this > automatically if the ‘token-parts’ property is set to > 0: > > http://www.webmproject.org/tools/encoder-parameters/#6-multi-threaded-encode-and-decode The ‘token-parts’ property is only relevant for splitting the coefficient data calculations across multiple data partitions, which I guess depends on the ‘threads’ property. However, the documentation says that an appropriate number of threads should be used automatically when decoding, but does not mention what happens for encoding, so I guess that this is not automatic.
(In reply to comment #2) > (In reply to comment #1) > > I do not think that Cheese should check the number of processors and calculate > > the number of threads to use, as the encoder is supposed to do this > > automatically if the ‘token-parts’ property is set to > 0: > > > > http://www.webmproject.org/tools/encoder-parameters/#6-multi-threaded-encode-and-decode > > The ‘token-parts’ property is only relevant for splitting the coefficient data > calculations across multiple data partitions, which I guess depends on the > ‘threads’ property. However, the documentation says that an appropriate number > of threads should be used automatically when decoding, but does not mention > what happens for encoding, so I guess that this is not automatic. Right, also notice how that section recommends to set threads and token-parts manually. And more importantly vp8enc defaults to 1 thread, not setting threads will always result in us using only one thread, which in turn gives quite poor performance.
(In reply to comment #3) > Right, also notice how that section recommends to set threads and token-parts > manually. And more importantly vp8enc defaults to 1 thread, not setting threads > will always result in us using only one thread, which in turn gives quite poor > performance. I think setting token-parts in Cheese based on the resolution is fine, but I do not think setting the threads based on the number of processors is fine. When the threads property was added to the vp8enc element in GStreamer, it was mentioned that the default setting should be 0, which would automatically select the number of threads based on the number of CPU cores: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/ext/vp8?id=7ce969720f20d4c58f70e96fc4aa53e2586c9e47 I think it is better to have this as the default behaviour in vp8enc, rather than each application doing the selection.
(In reply to comment #4) > (In reply to comment #3) > > Right, also notice how that section recommends to set threads and token-parts > > manually. And more importantly vp8enc defaults to 1 thread, not setting threads > > will always result in us using only one thread, which in turn gives quite poor > > performance. > > I think setting token-parts in Cheese based on the resolution is fine, but I do > not think setting the threads based on the number of processors is fine. When > the threads property was added to the vp8enc element in GStreamer, it was > mentioned that the default setting should be 0, which would automatically > select the number of threads based on the number of CPU cores True, that was mentioned, but quoting from the libvpx docs structvpx__codec__enc__cfg.html: "unsigned int vpx_codec_enc_cfg::g_threads Maximum number of threads to use. For multi-threaded implementations, use no more than this number of threads. The codec may use fewer threads than allowed. The value 0 is equivalent to the value 1." Note the value 0 == value 1, which means that such automatic select number of threads logic would need to be implemented at the gstreamer vp8enc element level, I don't think this belongs there, as whether to use multiple threads or not is something application specific (think realtime encoding versus offline encoding).. Also note how changing how the threads parameter works is not mentioned on the wishlist in bug 670108 .
(In reply to comment #5) > (In reply to comment #4) > > I think setting token-parts in Cheese based on the resolution is fine, but I do > > not think setting the threads based on the number of processors is fine. When > > the threads property was added to the vp8enc element in GStreamer, it was > > mentioned that the default setting should be 0, which would automatically > > select the number of threads based on the number of CPU cores > > True, that was mentioned, but quoting from the libvpx docs > structvpx__codec__enc__cfg.html: > > "unsigned int vpx_codec_enc_cfg::g_threads > > Maximum number of threads to use. > > For multi-threaded implementations, use no more than this number of threads. > The codec may use fewer threads than allowed. The value 0 is equivalent to the > value 1." > > Note the value 0 == value 1, which means that such automatic select number of > threads logic would need to be implemented at the gstreamer vp8enc element > level, I don't think this belongs there, as whether to > use multiple threads or not is something application specific (think realtime > encoding versus offline encoding). Cheese already uses the vp8enc realtime preset: http://git.gnome.org/browse/cheese/commit/?id=e7f9908166b64c6755437b825eb010806c043d21 so the automatic selection of the number of threads should be able to be represented in the preset, although currently that is not possible. I have no desire to add that code to Cheese, so consider that part of the patch rejected. Other GStreamer elements, such as x264enc, use an automatic number of threads if that property is set to 0: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-ugly-plugins/html/gst-plugins-ugly-plugins-x264enc.html#GstX264Enc--threads As there already seems to be some inconsistency in how different encoder elements deal with automatic selection of the number of threads, that can be fixed in GStreamer. > Also note how changing how the threads > parameter works is not mentioned on the wishlist > in bug 670108 . And yet that was specifically mentioned when the element was added to GStreamer. I am happy to let the GStreamer guys figure out what to do and come up with a useful realtime preset.
WONTFIX based on comment #6.