[Libva] [Libva-intel-driver][PATCH] Needn't reset brc if the bitrate setting isn't changed in the Begin/Render/End sequence

[Mark Thompson]

On 03/01/17 06:42, Xiang, Haihao wrote:
> On Fri, 2016-12-30 at 17:44 +0000, Mark Thompson wrote:
>> On 30/12/16 00:51, Xiang, Haihao wrote:
>>> On Thu, 2016-12-29 at 18:52 +0000, Mark Thompson wrote:
>>>> On 29/12/16 07:09, Zhao Yakui wrote:
>>>>> On 12/29/2016 03:08 PM, Xiang, Haihao wrote:
>>>>>>
>>>>>>> On 12/29/2016 01:22 PM, Xiang, Haihao wrote:
>>>>>>>> User can use VAEncMiscParameterRateControl to update
>>>>>>>> bitrate,
>>>>>>>> so we
>>>>>>>> should ignore
>>>>>>>> the bitrate in the sequence parameter if
>>>>>>>> VAEncMiscParameterRateControl is present.
>>>>>>>
>>>>>>> This makes sense. The MiscRateControl mentions that it can
>>>>>>> override
>>>>>>> the
>>>>>>> bps setting in sequence parameter.
>>>>>>>
>>>>>>>> Hence it is not needed to reset brc if
>>>>>>>> VAEncMiscParameterRateControl doesn't change
>>>>>>>> the used bitrate.
>>>>>>>
>>>>>>> Does it still need to send the
>>>>>>> VAEncMiscParameterRateControl
>>>>>>> parameter
>>>>>>> if the bps is not changed for the new sequence?
>>>>>>
>>>>>> Yes if bits_per_second in the sequence parameter is not equal
>>>>>> to
>>>>>> the
>>>>>> value for the previous Begin/Render/End sequence except
>>>>>> bits_per_second
>>>>>> in the sequence parameter is 0.
>>>>>>
>>>>>
>>>>> OK.
>>>>> It makes sense.
>>>>>
>>>>> This looks good to me.
>>>>
>>>> I'm not sure this behaviour is correct.  Should not the most
>>>> recently
>>>> given bitrate value from the user, either from sequence
>>>> parameters or
>>>> RC parameters, be used?  When is_new_sequence is set, the user
>>>> will
>>>> have provided a set of sequence parameters (because we are making
>>>> an
>>>> IDR frame), so the bitrate there is provided by the user and
>>>> should
>>>> override any previous RC parameters given before that frame (but
>>>> not
>>>> any on this one).
>>>>
>>>> That is, I think it should work like:
>>>>
>>>> Sequence parameters with bps = 1M
>>>> -> IDR frame, at 1Mbps
>>>> -> some P frames, at 1Mbps
>>>> RC parameters with bps = 2M
>>>> -> some P frames, at 2Mbps
>>>> Sequence parameters with bps = 3M
>>>> -> IDR frame, at 3Mbps
>>>> -> some P frames, at 3Mbps
>>>
>>>
>>> The 2nd sequence is generated at 3Mbps with the current
>>> logic. hl_bitrate_updated is 0 for your case and we use the
>>> sequence
>>> parameters.
>>
>> This is not the case - in the absence of a new RC parameter
>> structure, the one that was set in the middle of the first sequence
>> continues to apply forever (encoder_context->misc_param[] is never
>> cleared), so hl_bitrate_updated is always 1 after that point.
>>
> 
> You are right, I forgot encoder_context->misc_param[] 
> (except VAEncMiscParameterTypeROI) is not cleared presently. It would
> be better we use the same way for all misc parameters, I will file a
> patch to clear encoder_context->misc_param[]. 

Ok, sounds good :)

Thanks,

- Mark