GNOME Bugzilla – Bug 747534
vp8dec: add deadline option
Last modified: 2018-11-03 14:59:34 UTC
Created attachment 301163 [details] [review] Patch that adds deadline option to gstvp8dec This patch adds support for a "deadline" option in vp8dec, similar to that in gstvp8dec. This option is required to force the deadline to real-time (VPX_DL_REALTIME) in applications where real-time video is essential, e.g. in OpenWebRTC. I have tested and verified that the calculated deadline is actually 0 in the case of OpenWebRTC, and should be 1. By adding this option, the deadline can be overruled from the application end.
Review of attachment 301163 [details] [review]: I'm curious why the default doesn't work? It should try to push it in time for the sink ? ::: ext/vpx/gstvp8dec.c @@ +232,3 @@ + case PROP_DEADLINE: + dec->deadline = g_value_get_int64 (value); + break; Please also add the counterpart to get_property() @@ +528,3 @@ + if (dec->deadline) { + // Deadline is overwritten by property Pleaser no C++ comment, use /* */ comments instead. ::: ext/vpx/gstvp8dec.h @@ +39,3 @@ #endif +#include <vpx/vpx_encoder.h> Why include the encoder header here ?!
Review of attachment 301163 [details] [review]: ::: ext/vpx/gstvp8dec.c @@ +532,3 @@ } else { + // Automatically determine deadline + deadline = gst_video_decoder_get_max_decode_time (decoder, frame); Maybe this should be all automatic? If we're in a live pipeline, chances are good that we want realtime and nothing else. So maybe no need for a new property?
@slomo: Using the qos events to get the deadline from the sink is already what it does? I assume for some reason it doesn't work well enough ?
QoS does not work very well for live pipelines in my experience, but didn't look closer into that. But here it seemed to work well, it resulted in "0" as the deadline... but unfortunately 0 has a special meaning (no deadline IIRC, or something silly like that) in libvpx, and we want 1 (which has another special meaning: realtime). My suggestion was to use the live boolean from the LATENCY query here to always set 1 (aka REALTIME), but maybe we can also just to MAX(1, deadline) instead.
when interpreting the QoS, possibly one needs to take the latency into account in a live pipeline? Or possibly, just ignore the QoS entirely and derive the deadline from the latency, timestamp and the clock?
Sounds sensible, yes. Someone needs to implement that and do some testing to see what actually works best :)
Also see bug #640610 for latency and QoS events..
Created attachment 301194 [details] [review] rev 2 Patch has been updated.
See comment 4 and comment 5
-- 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/gst-plugins-good/issues/178.