[FFmpeg-devel] [PATCH 01/27] cbs_h2645: Merge SEI messages in common between codecs

[Mark Thompson]

On 04/01/2021 14:32, Nuo Mi wrote:
> On Sat, Jan 2, 2021 at 5:45 AM Mark Thompson <sw at jkqxz.net> wrote:
> 
>> Make a new template file for common SEI messages - this will also apply
>> to H.266.
>> ---
>>   libavcodec/cbs_h264.h                 |  37 +------
>>   libavcodec/cbs_h2645.c                |  24 +++--
>>   libavcodec/cbs_h264_syntax_template.c | 106 +-------------------
>>   libavcodec/cbs_h265.h                 |  45 ++-------
>>   libavcodec/cbs_h265_syntax_template.c | 135 ++++----------------------
>>   libavcodec/cbs_sei.h                  |  60 ++++++++++++
>>   libavcodec/cbs_sei_syntax_template.c  | 132 +++++++++++++++++++++++++
>>   libavcodec/h264_metadata_bsf.c        |   2 +-
>>   libavcodec/vaapi_encode_h264.c        |   2 +-
>>   libavcodec/vaapi_encode_h265.c        |  11 ++-
>>   10 files changed, 250 insertions(+), 304 deletions(-)
>>   create mode 100644 libavcodec/cbs_sei.h
>>   create mode 100644 libavcodec/cbs_sei_syntax_template.c
>>
>> ...
>> +
>> +static int FUNC(sei_user_data_registered)
>> +    (CodedBitstreamContext *ctx, RWContext *rw,
>> +     SEIRawUserDataRegistered *current, uint32_t *payload_size)
>> +{
>> +    int err, i, j;
>> +
>> +    HEADER("User Data Registered ITU-T T.35");
>> +
>> +    u(8, itu_t_t35_country_code, 0x00, 0xff);
>> +    if (current->itu_t_t35_country_code != 0xff)
>> +        i = 1;
>> +    else {
>> +        u(8, itu_t_t35_country_code_extension_byte, 0x00, 0xff);
>> +        i = 2;
>> +    }
>> +
>> +#ifdef READ
>> +    if (*payload_size < i) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR,
>> +               "Invalid SEI user data registered payload.\n");
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +    current->data_length = *payload_size - i;
>> +#else
>> +    *payload_size = i + current->data_length;
>> +#endif
>> +
>> +    allocate(current->data, current->data_length);
>> +    for (j = 0; j < current->data_length; j++)
>> +        xu(8, itu_t_t35_payload_byte[i], current->data[j], 0x00, 0xff, 1,
>> i + j);
>>
> although it's just a name, change  itu_t_t35_payload_byte[i] to
> itu_t_t35_payload_byte[i+j] maybe better?

No, because there is no standard variable called j so it shouldn't be in the string.  i makes the most sense given that i is the loop index, though it's really i - (1 or 2).

Maybe it would cause less confusion to leave the brackets empty?  (After all, that element isn't actually an array, we just write it as one here because it's cleaner.)

>> +
>> +    return 0;
>> +}
>> +

- Mark