Zhang, Boyuan wrote:
>>
>> 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@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.
>
> I guess you mean encoder rather than player - I don't think a player should
> care if there are no b-frames in main.
>
> FWIW vainfo does currently advertise main/high support, IIRC it was mentioned
> in the past but lost in a long thread with many other issues in it.
I remember there was a discussion before about this, I will double check it and
will probably make a patch to disable them if not yet disabled.
>
> 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.
>
> Maybe advertise constrained baseline as encode + expose cabac switch that's
> in the vaapi spec and let the user/app decide to force that.
If cabac is on then flag the output as main.
Thanks for this idea, I could raise this in our meeting to discuss the
possibility of doing this. Currently, "advertising constrained baseline only"
+ "enabling cabac" at the same time doesn't sound like a "correct"/"perfect"
way to me, I need more discussions with my team about this. Therefore, I will
push the current patches, e.g. keep cabac disabled for now, to bring up this
vcn encode if you are OK with it.
- Boyuan
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
