GNOME Bugzilla – Bug 784590
encoder: h264: Performance tuning for specific platforms
Last modified: 2017-08-01 18:48:25 UTC
Some of the small core Intel platform (eg: APL) doesn't have LLC and the driver using cflush to ensure data consistency which seems to be expensive, but we can still limit the impact by reducing the original size of coded-buffer from middleware. We always have to pre-calculate the coded-buffer-size and later driver copy the encoded data to this pre-allocated buffer.Currently, we pre-calculate buffer size based on worst case scenario. Even if we use the specification LevelLimits it will end up with a maximum possible value. But we can do some optimization based on MinCR field in AnnexA which helps to reduce the buffer size. This is not strictly following the specification. The idea is to add a new "compliance-mode" enum property to vaapih264enc. So that we can add other tweaks under this single property in future.
Created attachment 354984 [details] [review] libs: utils_h264: Extend LevelLimit table with MinCR field
Created attachment 354985 [details] [review] libs: encoder: h264: Add less restrictive compliance mode to reduce the coded buffer size
Review of attachment 354984 [details] [review]: lgtm
Review of attachment 354985 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapiencoder_h264.c @@ +98,3 @@ + {GST_VAAPI_ENCODER_H264_COMPLIANCE_MODE_RESTRICT_CODED_BUFFER_ALLOC, + "Restict the allocation size of coded-buffer", + "restrict-buf-alloc"} the array needs a null structure {0,NULL,NULL} otherwise the code might crash @@ +2925,3 @@ + if (encoder->compliance_mode == + GST_VAAPI_ENCODER_H264_COMPLIANCE_MODE_RESTRICT_CODED_BUFFER_ALLOC) + base_encoder->codedbuf_size /= encoder->min_cr; I don't like having a property for this. would be possible to query the driver to know if this is required, avoiding the property?
(In reply to Víctor Manuel Jáquez Leal from comment #4) > Review of attachment 354985 [details] [review] [review]: > > ::: gst-libs/gst/vaapi/gstvaapiencoder_h264.c > @@ +98,3 @@ > + {GST_VAAPI_ENCODER_H264_COMPLIANCE_MODE_RESTRICT_CODED_BUFFER_ALLOC, > + "Restict the allocation size of coded-buffer", > + "restrict-buf-alloc"} > > the array needs a null structure > {0,NULL,NULL} > > otherwise the code might crash I will fix it, thanks! > > @@ +2925,3 @@ > + if (encoder->compliance_mode == > + GST_VAAPI_ENCODER_H264_COMPLIANCE_MODE_RESTRICT_CODED_BUFFER_ALLOC) > + base_encoder->codedbuf_size /= encoder->min_cr; > > I don't like having a property for this. I too, but I can't find a better way to do this. A property named "restrict-coded-buf-size" is another option. But we may add more of this kind of properties in future, specific to some hardware for eg. May be it is better to consolidate everything under a single property "compliance-mode" like we did for vaapih264dec? > would be possible to query the driver to know if this is required, avoiding > the property? Not possible. This is not related with libva at all. In theory, using the min-compression-ratio outside of the equations provided in the spec is not correct, but we are only considering intel-vaapi-driver here and which should work with the new size calculation. So in nutshell, it is a *tweak* which we don't want to enable by default, but on the other hand, many of the customers don't care about specification/implementation as long as they get better performance :). Software like yami is allocating even lower size for coded-buffer which is something I don't want to do. So enabling this under a property seems to be a fair compromise IMHO.
Created attachment 355978 [details] [review] libs: encoder: h264: Add less restrictive compliance mode to reduce the coded buffer size
Attachment 354984 [details] pushed as f775bbf - libs: utils_h264: Extend LevelLimit table with MinCR field Attachment 355978 [details] pushed as aa35439 - libs: encoder: h264: Add uncompliant mode reducing coded buffer size (modified a bit the commit log)
Thanks !