[FFmpeg-devel] Request for review - x265 User Data Unregistered SEI patch

[Timo Rothenpieler]

On 11.07.2021 14:01, Derek Buitenhuis wrote:
> Hi Brad,
> 
> On 7/8/2021 4:31 AM, Brad Hards wrote:
>> About a month ago, I submitted a patch to add User Data Unregistered SEI
>> writing to the x265 implementation.
>>
>> See http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/280978.html[1]
>> and
>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210605102028.15571-2-bradh@frogmouth.net/[2]
>>
>> If this is OK, can it please be merged? If not, can I get feedback so I can address the issues?
> 
> Can you amend the commit message to contain the reasoning from [1]?
> 
> A quick review:
> 
>> +    void *sei_data;
>> +    int sei_data_size;
> 
> I don't see sei_data freed anywhere at the end of decoding?
> 
>>       if (pic) {
>> +        x265_sei *sei = &(x265pic.userSEI);
> 
> Drop the paren for consistency with the rest of the codebase.
> 
>> +            tmp = av_fast_realloc(ctx->sei_data,
>> +                                  &ctx->sei_data_size,
>> +                                  (sei->numPayloads + 1) * sizeof(x265_sei_payload));
> 
> Convention in FFmpeg is to do sizeof(*var).
> 
>> +            if (!tmp) {
>> +                av_freep(&x265pic.userData);
>> +                av_freep(&x265pic.quantOffsets);
>> +                return AVERROR(ENOMEM);
>> +            } else {
> 
> This else statement is not needed.
> 
>> +                sei_payload = &(sei->payloads[sei->numPayloads]);
> 
> Drop the paren.
> 
>> +                sei_payload->payloadType = USER_DATA_UNREGISTERED;
> 
> I'm surprised x265 has un-namespaced enums... gross.

Could probably use our SEI_TYPE_USER_DATA_UNREGISTERED instead, seems to 
refer to the same value.

> - Derek
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4494 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210711/278b7e8c/attachment.bin>