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 719694 - encoder: h264: allow target decoder constraints
encoder: h264: allow target decoder constraints
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Wind Yuan
gstreamer-vaapi maintainer(s)
Depends on: 719693 719827
Blocks: 719412 719707
 
 
Reported: 2013-12-02 16:10 UTC by Gwenole Beauchesne
Modified: 2014-01-13 16:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0003-encoder-h264-constrain-profile-and-level-by-caps (17.61 KB, patch)
2013-12-19 09:47 UTC, Wind Yuan
none Details | Review
0002-encoder-h264-expose-cabac-dct8x8 (7.32 KB, patch)
2013-12-19 09:49 UTC, Wind Yuan
none Details | Review
0001-h264-check-constrained-profile-for-encoder (2.61 KB, patch)
2013-12-19 09:50 UTC, Wind Yuan
none Details | Review
0003-encoder-h264-constrain-profile-and-level-by-caps (16.10 KB, patch)
2013-12-24 08:48 UTC, Wind Yuan
none Details | Review

Description Gwenole Beauchesne 2013-12-02 16:10:48 UTC
This bug addresses requirements to limit profile and level, based on target decoder constraints. The constraints are to be specified through downstream caps only, not vaapiencode_h264 properties.

* Public API:
- gst_vaapi_encoder_h264_set_max_profile_and_level().

* Order of resolution:
- Plug-in element (vaapiencode_h264) settings are applied first ;
- Then, downstream element caps are applied through the public API.

This means that H.264 coding tools are changed accordingly. e.g. if "cabac" is enabled and that downstream caps mention profile=constrained-baseline, then "cabac" is set to FALSE implicitly.
Comment 1 Wind Yuan 2013-12-13 04:40:01 UTC
good to see profile/level opened. I'll check this bug.
Comment 2 Wind Yuan 2013-12-19 09:47:20 UTC
Created attachment 264513 [details] [review]
0003-encoder-h264-constrain-profile-and-level-by-caps


encoder h264: constrain profile and level by caps
     
     If capsfilter set profile and level, encoder need calculate
     profile/level <= limits. if some propoerty breaks max
     profile, need to reset the property and make profile/level
     first priority.

This patch depends on 719827 and 719693
Comment 3 Wind Yuan 2013-12-19 09:49:33 UTC
Created attachment 264514 [details] [review]
0002-encoder-h264-expose-cabac-dct8x8

this patch going to fix 719693, attached here in case to make owner review and merge easily
Comment 4 Wind Yuan 2013-12-19 09:50:49 UTC
Created attachment 264515 [details] [review]
0001-h264-check-constrained-profile-for-encoder

this patch going to fix 719827, attached here in case to make owner review and
merge easily
Comment 5 Wind Yuan 2013-12-19 09:53:50 UTC
After the 3 patches. tests passed by
1. gst-launch-1.0 filesrc  ! videoparse  ! vaapiencode_h264 max-bframes=2 dct8x8=1 ! video/x-h264,profile="constrained-baseline" ! filesink location=1.264
0:00:00.180908231 28558  0x8aff630 WARN                   vaapi gstvaapiencoder_h264.c:379:_apply_profile: b_frame_num reset to 0 since profile limits
0:00:00.180982109 28558  0x8aff630 WARN                   vaapi gstvaapiencoder_h264.c:388:_apply_profile: dct8x8 reset to FALSE since profile limits

2. gst-launch-1.0 filesrc  ! videoparse  ! vaapiencode_h264 ! video/x-h264,profile="main" ! filesink location=1.264
 the output data with profile "constrained-baseline"

profile="" going to limits the max profile.
Comment 6 Gwenole Beauchesne 2013-12-20 09:32:20 UTC
Review of attachment 264513 [details] [review]:

GstVaapiEncoder should be using GstVaapiProfile and GstVaapiLevelH264 utilities.

::: gst-libs/gst/vaapi/gstvaapiencoder_h264.c
@@ +154,3 @@
+
+static inline gboolean
+_get_profiles_from_structure (GstStructure *structure, GArray *profiles)

Is this really needed? At this point, the caps should be fixated.

@@ +196,3 @@
+  value = gst_vaapi_utils_h264_get_profile_idc (profile);
+  if (profile == GST_VAAPI_PROFILE_H264_CONSTRAINED_BASELINE)
+    --value;

I don't think this is right. profile_idc is still 66 in either baseline or constrained-baseline profiles.

@@ +402,3 @@
+  GstVaapiProfile profiles[10];
+  GstVaapiProfile best_profile = GST_VAAPI_PROFILE_UNKNOWN;
+  gint i, n = 0;

Ah, you implemented here what I mentioned in another patch. :)
Comment 7 Wind Yuan 2013-12-23 08:10:41 UTC
(In reply to comment #6)
> Review of attachment 264513 [details] [review]:
> 
> GstVaapiEncoder should be using GstVaapiProfile and GstVaapiLevelH264
> utilities.
I saw you add GstVaapiLevelH264, but if switch to GstVaapiLevelH264 from this patch, need set_level and others do the whole change. suggest do the change in another patch together. not in this patch.

> 
> ::: gst-libs/gst/vaapi/gstvaapiencoder_h264.c
> @@ +154,3 @@
> +
> +static inline gboolean
> +_get_profiles_from_structure (GstStructure *structure, GArray *profiles)
> 
> Is this really needed? At this point, the caps should be fixated.
The caps was fixated after set_format. so there's possible have a profile list by the downstream caps. I'm trying to find the max profile. and this is also the APIs with *max* you defined
gst_vaapi_encoder_h264_set_max_profile_and_level(). If you think no need to get max profile but gst_vaapi_encoder_h264_set_profile(), I think I only need to get a fixated caps and directly set to GstVaapiEncoderH264.
How do you think about?

> 
> @@ +196,3 @@
> +  value = gst_vaapi_utils_h264_get_profile_idc (profile);
> +  if (profile == GST_VAAPI_PROFILE_H264_CONSTRAINED_BASELINE)
> +    --value;
> 
> I don't think this is right. profile_idc is still 66 in either baseline or
> constrained-baseline profiles.
I know. this is only for compare profile to get a max proflie(to make constrained-baseline < baseline < main < high), that's why I used get_compareble_proifle_value.

> 
> @@ +402,3 @@
> +  GstVaapiProfile profiles[10];
> +  GstVaapiProfile best_profile = GST_VAAPI_PROFILE_UNKNOWN;
> +  gint i, n = 0;
> 
> Ah, you implemented here what I mentioned in another patch. :)
Ok, I'll consider patch in here or #719827
Comment 8 Wind Yuan 2013-12-24 08:48:25 UTC
Created attachment 264832 [details] [review]
0003-encoder-h264-constrain-profile-and-level-by-caps

update since the 2 depended patch changes. please review.

depends on 
https://bugzilla.gnome.org/review?bug=719827&attachment=264830
https://bugzilla.gnome.org/review?bug=719693&attachment=264831

please comments. GstVaapiLevel better to change in a separate patch. anything you think different of set_max_profile, please comments.
and also I suggest to separate profile and level into 2 APIs and not use set_max_xxx but set_profile and set_level. It's better to generate a stream user exactly wanted.
Comment 9 Gwenole Beauchesne 2014-01-13 16:39:54 UTC
commit bdf91aa765a781dba35606b63aff8264edd203ad
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Sun Jan 12 22:24:04 2014 +0100

    encoder: h264: allow target decoder constraints.
    
    Allow user to precise the largest profile to use for encoding due
    to target decoder constraints. For instance, if CABAC entropy coding
    mode is requested by "constrained-baseline" profile only is desired,
    then an error is returned during codec configuration.
    
    Also make sure that the suitable profile we derived actually matches
    what the HW can cope with.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=719694