[Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations

[Zhang, Boyuan]

On 14/11/17 10:53, Andy Furniss wrote:
>> Zhang, Boyuan wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Zhang, Boyuan
>>> Sent: November-13-17 11:41 AM
>>> To: Andy Furniss; Koenig, Christian; Mark Thompson; mesa-dev at lists.freedesktop.org
>>> Subject: RE: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations
>>>
>>> Zhang, Boyuan wrote:
>>>
>>>>>>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> index 5170c67..c6dc420 100644
>>>>>>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> @@ -362,6 +362,233 @@ static void radeon_enc_quality_params(struct radeon_encoder *enc)
>>>>>>>>         RADEON_ENC_END();
>>>>>>>>     }
>>>>>>>>     +static void radeon_enc_nalu_sps(struct radeon_encoder *enc) {
>>>>>>>> +    RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
>>>>>>>> +    RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
>>>>>>>> +    uint32_t *size_in_bytes = &enc->cs->current.buf[enc->cs->current.cdw++];
>>>>>>>> +    radeon_enc_reset(enc);
>>>>>>>> +    radeon_enc_set_emulation_prevention(enc, false);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, 0x00000001, 32);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, 0x67, 8);
>>>>>>>> +    radeon_enc_byte_align(enc);
>>>>>>>> +    radeon_enc_set_emulation_prevention(enc, true);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, enc->enc_pic.spec_misc.profile_idc, 8);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, 0x04, 8);
>>>>>>> Please always set constraint_set1_flag when profile_idc is 66.  There are enough actually-constrained-baseline-but-not-marked-as-such streams in the world already to catch out decoders without full baseline support (that is, all of them).
>>>>>>>
>>>>>>> Also, "constraint_set5_flag shall be equal to 0 when profile_idc is not equal to 77, 88, 100, or 118".
>>>>>
>>>>> [Boyuan] Thanks for pointing out. I modified the value to be 0x44 in the new patch (set1=1, and set5=1) since we only support constrained baseline for now.
>>>>
>>>> It's not really with cabac though. I know there was a patch to turn it off - but that would have been wasteful and make linux < windows.
>>>>
>>>> Why not use 77 if cabac is on + not set constrained bits, windows seems to set main.
>>>>
>>>> Currently with vce trying to set main manually from ffmeg/gst in order to get something "correct" still sets flags = something that's not seen as main (but works).
>>>
>>> Yes, but still the problem is cabac is not allowed for baseline profile and we only support baseline for now. I'm not quite sure about windows test you mentioned, I'm just guessing that we might have some main profile features support in some closed test environment on windows side, but definitely not all main profile features, no b frame support.
>>> On linux side, we only support baseline profile for this vcn enc.
>>> (fixed a typo)
>> 
>>  Yea, it's tricky, FWIW windows does not use b-frames for the test I've done (re-live turned up high = 50mbit 60fps).
>
> I can get B-frames from AMF with a GCN 2 card on Windows.

Yes, we used to support b-frame for GCN2 (e.g. like Bonaire, Kabini, etc...) cards. But for later Asics, we dropped the b-frame support. Including this raven vcn encode, b-frame is still not supported according to our firmware team.
- Boyuan

>
>> It is flagged as main and uses cabac.
>> I guess main is the "correct" way to describe constrained baseline + cabac even if there are no other main features like b-frames.

Yes, I agree, that's the correct way in that case. However, we can't advertise main profile since we don't have b-frame support. It would cause serious problem if player tries to use b-frame when seeing that main profile is supported. So as explained before, we only support constrained baseline profile for this raven vcn encode now. And as a result, cabac is forced to be off in this bring-up patch.
- Boyuan

>
> Yes, it must be flagged as main in this case.
>
> - Mark