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

[Brad Hards]

On Sunday, 11 July 2021 10:01:47 PM AEST Derek Buitenhuis wrote:
> Can you amend the commit message to contain the reasoning from [1]?
Amended.

> A quick review:
> > +    void *sei_data;
> > +    int sei_data_size;
> 
> I don't see sei_data freed anywhere at the end of decoding?
Fixed in v2. Included in the _close().
 
> >      if (pic) {
> > 
> > +        x265_sei *sei = &(x265pic.userSEI);
> 
> Drop the paren for consistency with the rest of the codebase.
Fixed in v2.
 
> > +            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).
Fixed in v2.

> > +            if (!tmp) {
> > +                av_freep(&x265pic.userData);
> > +                av_freep(&x265pic.quantOffsets);
> > +                return AVERROR(ENOMEM);
> > +            } else {
> 
> This else statement is not needed.
Fixed in v2.
 
> > +                sei_payload = &(sei->payloads[sei->numPayloads]);
> 
> Drop the paren.
Fixed in v2.
 
> > +                sei_payload->payloadType = USER_DATA_UNREGISTERED;
> 
> I'm surprised x265 has un-namespaced enums... gross.
I took Timo's suggest in v2, although I conceptually wanted to say "the x265 value". So 
there is a comment.

Brad