[FFmpeg-devel] [PATCH 2/3] libavformat/hls: add support for SAMPLE-AES decryption in HLS demuxer

[James Almer]

On 1/28/2021 12:53 PM, Lynne wrote:
> Jan 28, 2021, 16:11 by nachiket.programmer at gmail.com:
>> +static int decrypt_nal_unit(HLSCryptoContext *crypto_ctx, NALUnit *nalu)
>> +{
>> +    int ret = 0;
>> +    int rem_bytes;
>> +    uint8_t *data;
>> +    uint8_t iv[16];
>> +
>> +    ret = av_aes_init(crypto_ctx->aes_ctx, crypto_ctx->key, 16 * 8, 1);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    /* Remove start code emulation prevention 0x03 bytes */
>> +    remove_scep_3_bytes(nalu);
>> +
>> +    data = nalu->data + 32;
>> +    rem_bytes = nalu->length - 32;
>> +
>> +    memcpy(iv, crypto_ctx->iv, 16);
>> +
>> +    while (rem_bytes > 0) {
>> +        if (rem_bytes > 16) {
>> +            av_aes_crypt(crypto_ctx->aes_ctx, data, data, 1, iv, 1);
>> +            data += 16;
>> +            rem_bytes -= 16;
>> +        }
>> +        data += FFMIN(144, rem_bytes);
>> +        rem_bytes -= FFMIN(144, rem_bytes);
>> +    }
>>
> 
> We do not put brackets on single-line statements like
> for (int)
>      do_thing();
> or
> while (1)
>      do_thing()
> or
> if (1)
>      do_thing()
> or
> if (1)
>      do_thing1()
> else
>      do_thing2()
> 
> But when part of another block which contains more than one line like
> if (1) {
>      do_thing1();
>      do_thing2();
> } else {
>      do_thing3();
> }
> we do put brackets on all blocks. We even have a document with the
> coding style which says what to do and what not to do.
> But even then, the rest of the libavformat/hls.c file follows our style,
> so you could have at least looked at the file you were working on to
> see this.

Could we focus on a technical review of these patches? Cosmetics are 
last in the list of priorities.

I'm not saying that what you said is not true or that it does not need 
to be fixed, but when you ask for several iterations just to fix a 
single space or a line break, and only then start looking at the 
implementation, which may need even more iterations, it can get frustrating.

So what I'm asking is, is the patch correct from a technical PoV? Are 
the only issues left purely cosmetics? If not, point everything so it 
may all be fixed in a single iteration.