[FFmpeg-devel] [PATCH 4/6] Support encoding of Active Format Description (AFD) in libx264

[Derek Buitenhuis]

On 11/16/2017 6:34 PM, Devin Heitmueller wrote:

> +        /* Active Format Description */
> +        if (x4->afd) {
> +            void *sei_data;
> +            size_t sei_size;
> +
> +            ret = ff_alloc_afd_sei(frame, 0, &sei_data, &sei_size);
> +            if (ret < 0) {
> +                av_log(ctx, AV_LOG_ERROR, "Not enough memory for AFD, skipping\n");
> +            } else if (sei_data) {

In an OOM situation, we always fail hard.

> +                x4->pic.extra_sei.payloads = av_realloc(x4->pic.extra_sei.payloads,
> +                                                        sizeof(x4->pic.extra_sei.payloads[0]) * (num_payloads + 1));
> +                if (x4->pic.extra_sei.payloads == NULL) {
> +                    av_log(ctx, AV_LOG_ERROR, "Not enough memory for AFD, skipping\n");

This will leak the original x4->pic.extra_sei.payloads on failure, won't it?

Also, as above, we should fail hard here.

> +    /* country code (SCTE 128-1 Sec 8.1.1) */
> +    sei_data[0] = 181;
> +    sei_data[1] = 0;
> +    sei_data[2] = 49;
> +
> +    /* country code (SCTE 128-1 Sec 8.1.2) */
> +    AV_WL32(sei_data + 3, MKTAG('D', 'T', 'G', '1'));
> +
> +    /* country code (SCTE 128-1 Sec 8.2.5) */
> +    sei_data[7] = 0x41;
> +    sei_data[8] = 0xf0 | side_data->data[0];

I assume these values are supposed to always be the same? Excuse my unfamiliarity
with SCTE-128 - country codes sounds like something you wouldn't want to hardcode.

- Derek