GNOME Bugzilla – Bug 719693
encoder: h264: expose more coding tools
Last modified: 2014-01-13 16:39:32 UTC
In view to precisely expose (and test) key features of the H.264 encoder, the following properties could be added: - "cabac": enable CABAC entropy coding (default: FALSE) ; - "dct8x8": enable spatial transform 8x8 (default: FALSE). Note: - If "cabac" is set to TRUE, the profile is inferred to be Main ; - If "dct8x8" is set to TRUE, the profile is inferred to be High.
I'll take look of this bug.
Created attachment 264128 [details] [review] 0002-encoder-h264-expose-cabac-dct8x8 patch attached. please review. this patch depends on https://bugzilla.gnome.org/attachment.cgi?id=264048 bug https://bugzilla.gnome.org/show_bug.cgi?id=719827
Review of attachment 264128 [details] [review]: Overall, looks OK but for the public interface. ::: gst-libs/gst/vaapi/gstvaapiencoder_h264.h @@ +51,3 @@ +gboolean +gst_vaapi_encoder_h264_get_dct8x8_mode (GstVaapiEncoderH264 * encoder); + Please see bug #719529. That's why single approach through gst_vaapi_encoder_h264_{get,set}_params is simpler. :)
(In reply to comment #3) > Review of attachment 264128 [details] [review]: > > Overall, looks OK but for the public interface. > > ::: gst-libs/gst/vaapi/gstvaapiencoder_h264.h > @@ +51,3 @@ > +gboolean > +gst_vaapi_encoder_h264_get_dct8x8_mode (GstVaapiEncoderH264 * encoder); > + > > Please see bug #719529. That's why single approach through > gst_vaapi_encoder_h264_{get,set}_params is simpler. :) I think {get,set}_params should be fix in #719529 with other params together. not in this bug.
Created attachment 264831 [details] [review] 0002-encoder-h264-expose-cabac-dct8x8 update patch since depends on https://bugzilla.gnome.org/review?bug=719827&attachment=264830
(In reply to comment #4) > (In reply to comment #3) > > Review of attachment 264128 [details] [review] [details]: > > > > Overall, looks OK but for the public interface. > > > > ::: gst-libs/gst/vaapi/gstvaapiencoder_h264.h > > @@ +51,3 @@ > > +gboolean > > +gst_vaapi_encoder_h264_get_dct8x8_mode (GstVaapiEncoderH264 * encoder); > > + > > > > Please see bug #719529. That's why single approach through > > gst_vaapi_encoder_h264_{get,set}_params is simpler. :) > > I think {get,set}_params should be fix in #719529 with other params together. > not in this bug. Yes, that's why this bug was marked as depending on bug #719529. I did not mean to have public interfaces for dct8x8 and cabac, except through the params mechanism. Despite that the patch looks OK. Edited to fit newer tree.
ok, I'll update when you fixed bug #719529
Wind, could you please check with the driver team why "dct8x8" = TRUE has no influence on the compression ratio? Do we need to do something else at the middleware level but setting transform_8x8_mode_flag to 1?
(In reply to comment #8) > Wind, could you please check with the driver team why "dct8x8" = TRUE has no > influence on the compression ratio? Do we need to do something else at the > middleware level but setting transform_8x8_mode_flag to 1? Hi Gwenole, just checked with Haihao. There's very little influence of dct8x8 for coding. and compare from spec. it only affect I_NXN and when transform_size_8x8_flag equal to 1. QA also said MediaSDK doesn't have an option for dct8x8, maybe the same reason.
commit 8df97cf9ec600d8920de43dbaa75be940c669f05 Author: Wind Yuan <feng.yuan@intel.com> Date: Fri Dec 13 17:36:08 2013 +0800 encoder: h264: expose more coding tools. Add new H.264 coding tools to improve compression: - "cabac": enable CABAC entropy coding (default: FALSE); - "dct8x8": enable spatial transform 8x8 (default: FALSE). https://bugzilla.gnome.org/show_bug.cgi?id=719693 Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>