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. To show that more concretely, with this patch applied to print the internal state: diff --git a/src/i965_encoder.c b/src/i965_encoder.c index 0a648d4..a7c92ce 100644 --- a/src/i965_encoder.c +++ b/src/i965_encoder.c @@ -718,6 +718,12 @@ intel_encoder_check_brc_parameter(VADriverContextP ctx, } + printf("hl_bitrate_updated = %d, is_new_sequence = %d, need_reset = %d, " + "seq_bits_per_second = %d, brc bits_per_second = %d\n", + hl_bitrate_updated, encoder_context->is_new_sequence, + encoder_context->brc.need_reset, seq_bits_per_second, + encoder_context->brc.bits_per_second[0]); + return VA_STATUS_SUCCESS; } I then encode with a GOP size of 2 and and bitrate increasing by 1M on each IDR frame, providing RC parameters only with the first: hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 1, seq_bits_per_second = 1000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 2000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 3000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 4000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 5000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 6000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 7000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 8000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 9000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 1, need_reset = 0, seq_bits_per_second = 10000000, brc bits_per_second = 1000000 hl_bitrate_updated = 1, is_new_sequence = 0, need_reset = 0, seq_bits_per_second = 0, brc bits_per_second = 1000000 Thanks, - Mark _______________________________________________ Libva mailing list Libva@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libva