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

[Derek Buitenhuis]

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.

- Derek