munmap_chunk(): invalid pointer encoding SEI with libx265

[Brad Hards]

I'm seeing a problem encoding User Data Unregistered SEI messages with libx265 that I don't see in libx264. I'm seeing it via Java bindings (javacpp-presets), but I think I'm seeing something similar in this modified example:

/*
 * Copyright (c) 2001 Fabrice Bellard
 *
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 * THE SOFTWARE.
 */

/**
 * @file
 * video encoding with SEI (user unregistered) API example
 *
 * @example encode_unregistered.c
 */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include <libavcodec/avcodec.h>

#include <libavutil/opt.h>
#include <libavutil/imgutils.h>

static void encode(AVCodecContext *enc_ctx, AVFrame *frame, AVPacket *pkt,
                   FILE *outfile)
{
    int ret;

    /* send the frame to the encoder */
    if (frame)
        printf("Send frame %3" PRId64 "\n", frame->pts);

    ret = avcodec_send_frame(enc_ctx, frame);
    if (ret < 0)
    {
        fprintf(stderr, "Error sending a frame for encoding\n");
        exit(1);
    }

    while (ret >= 0)
    {
        ret = avcodec_receive_packet(enc_ctx, pkt);
        if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
            return;
        else if (ret < 0)
        {
            fprintf(stderr, "Error during encoding\n");
            exit(1);
        }

        printf("Write packet %3" PRId64 " (size=%5d)\n", pkt->pts, pkt->size);
        fwrite(pkt->data, 1, pkt->size, outfile);
        av_packet_unref(pkt);
    }
}

int main(int argc, char **argv)
{
    const char *filename, *codec_name;
    const AVCodec *codec;
    AVCodecContext *c = NULL;
    int i, ret, x, y;
    FILE *f;
    AVFrame *frame;
    AVPacket *pkt;
    AVBufferRef *ref;
    AVFrameSideData *side_data;
    char *tempBuf;
    // This is from MISB ST 2101
    char sei_message[] = {0xa5, 0x50, 0x52, 0xaf, 0x52, 0x16, 0x5f, 0x45,
                          0xa3, 0x18, 0x1c, 0xfc, 0x7a, 0xbb, 0xc2, 0x67,
                          0x01, 0x70, 0xF5, 0x92, 0xF0, 0x23, 0x73, 0x36,
                          0x4A, 0xF8, 0xAA, 0x91, 0x62, 0xC0, 0x0F, 0x2E,
                          0xB2, 0xDA, 0x16, 0xB7, 0x43, 0x41, 0x00, 0x08,
                          0x41, 0xA0, 0xBE, 0x36, 0x5B, 0x5A, 0xB9, 0x6A,
                          0x36, 0x45};

    if (argc <= 2) {
        fprintf(stderr, "Usage: %s <output file> <codec name>\n", argv[0]);
        exit(0);
    }
    filename = argv[1];
    codec_name = argv[2];

    codec = avcodec_find_encoder_by_name(codec_name);
    if (!codec) {
        fprintf(stderr, "Codec '%s' not found\n", codec_name);
        exit(1);
    }

    c = avcodec_alloc_context3(codec);
    if (!c)
    {
        fprintf(stderr, "Could not allocate video codec context\n");
        exit(1);
    }

    pkt = av_packet_alloc();
    if (!pkt)
        exit(1);

    c->bit_rate = 400000;
    c->width = 352;
    c->height = 288;
    /* frames per second */
    c->time_base = (AVRational){1, 25};
    c->framerate = (AVRational){25, 1};
    c->gop_size = 10;
    c->max_b_frames = 0;
    c->pix_fmt = AV_PIX_FMT_YUV420P;
    if (codec->id == AV_CODEC_ID_H264) {
        av_opt_set(c->priv_data, "preset", "medium", 0);
        
    }
    av_opt_set(c->priv_data, "udu_sei", "1", 0);
    
    /* open it */
    ret = avcodec_open2(c, codec, NULL);
    if (ret < 0)
    {
        fprintf(stderr, "Could not open codec: %s\n", av_err2str(ret));
        exit(1);
    }

    f = fopen(filename, "wb");
    if (!f)
    {
        fprintf(stderr, "Could not open %s\n", filename);
        exit(1);
    }

    frame = av_frame_alloc();
    if (!frame)
    {
        fprintf(stderr, "Could not allocate video frame\n");
        exit(1);
    }
    frame->format = c->pix_fmt;
    frame->width = c->width;
    frame->height = c->height;

    ret = av_frame_get_buffer(frame, 0);
    if (ret < 0)
    {
        fprintf(stderr, "Could not allocate the video frame data\n");
        exit(1);
    }

    /* encode 1 second of video */
    for (i = 0; i < 25; i++)
    {
        fflush(stdout);

        /* make sure the frame data is writable */
        ret = av_frame_make_writable(frame);
        if (ret < 0)
            exit(1);
        side_data = av_frame_new_side_data(frame, AV_FRAME_DATA_SEI_UNREGISTERED, sizeof(sei_message));
        if (!side_data)
        {
            fprintf(stderr, "Could not allocate the video frame side data\n");
            exit(1);
        }
        memcpy(side_data->data, sei_message, side_data->size);

        /* prepare a dummy image */
        /* Y */
        for (y = 0; y < c->height; y++) {
            for (x = 0; x < c->width; x++) {
                frame->data[0][y * frame->linesize[0] + x] = x + y + i * 3;
            }
        }

        /* Cb and Cr */
        for (y = 0; y < c->height/2; y++) {
            for (x = 0; x < c->width/2; x++) {
                frame->data[1][y * frame->linesize[1] + x] = 128 + y + i * 2;
                frame->data[2][y * frame->linesize[2] + x] = 64 + x + i * 5;
            }
        }

        frame->pts = i;
        encode(c, frame, pkt, f);
    }

    /* flush the encoder */
    encode(c, NULL, pkt, f);
    fclose(f);

    avcodec_free_context(&c);
    av_frame_free(&frame);
    av_packet_free(&pkt);

    return 0;
}

The backtrace for that looks like:

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
#1  0x00007ffff684d864 in __GI_abort () at abort.c:79
#2  0x00007ffff68b0736 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff69d5b9c "%s\n") at ../sysdeps/posix/libc_fatal.c:155
#3  0x00007ffff68b908c in malloc_printerr (str=str@entry=0x7ffff69d7ba0 "munmap_chunk(): invalid pointer") at malloc.c:5628
#4  0x00007ffff68b945c in munmap_chunk (p=<optimised out>) at malloc.c:2995
#5  0x00007ffff68be7db in __GI___libc_free (mem=<optimised out>) at malloc.c:3302
#6  0x00007ffff6acecac in x265::NALList::takeContents(x265::NALList&) () from /lib/x86_64-linux-gnu/libx265.so.192
#7  0x00007ffff6aca817 in x265::FrameEncoder::getEncodedPicture(x265::NALList&) () from /lib/x86_64-linux-gnu/libx265.so.192
#8  0x00007ffff6af88bf in x265::Encoder::encode(x265_picture const*, x265_picture*) () from /lib/x86_64-linux-gnu/libx265.so.192
#9  0x00007ffff6afbce0 in x265_encoder_encode () from /lib/x86_64-linux-gnu/libx265.so.192
#10 0x000055555584a342 in libx265_encode_frame (avctx=0x555556a60d80, pkt=0x555556a62bc0, pic=<optimised out>, got_packet=0x7fffffffdab4) at libavcodec/libx265.c:576
#11 0x0000555555723241 in encode_simple_internal (avpkt=0x555556a62bc0, avctx=0x555556a60d80) at libavcodec/encode.c:211
#12 encode_simple_receive_packet (avpkt=<optimised out>, avctx=<optimised out>) at libavcodec/encode.c:266
#13 encode_receive_packet_internal (avctx=avctx@entry=0x555556a60d80, avpkt=avpkt@entry=0x555556a62bc0) at libavcodec/encode.c:300
#14 0x0000555555723962 in avcodec_receive_packet (avctx=avctx@entry=0x555556a60d80, avpkt=avpkt@entry=0x555556a62bc0) at libavcodec/encode.c:401
#15 0x000055555562d2a1 in encode (enc_ctx=0x555556a60d80, frame=frame@entry=0x0, pkt=0x555556a62bc0, outfile=outfile@entry=0x555556a5feb0)
    at doc/examples/encode_unregistered.c:57
#16 0x000055555562cf7b in main (argc=<optimised out>, argv=<optimised out>) at doc/examples/encode_unregistered.c:202

I had some involvement in the development of those, and ​https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/66f8055c898887c33ab124ca5f00ee60bf5fcf19 for libx264 looks like it deals with the underlying design issue - libx264 doesn't cause the backtrace. Possibly something similar would be needed for libx265.

[mkver]

Your code adds ever more AV_FRAME_DATA_SEI_UNREGISTERED side data to the frame (the old ones are not discarded). And libx265's behaviour upon encountering this is buggy. Here is the relevant code that copies the input NAL units to the internal frames used by libx265:

    int numPayloads = pic_in->userSEI.numPayloads + toneMapPayload + userPayload;
    frame->m_userSEI.numPayloads = numPayloads;

    if (frame->m_userSEI.numPayloads)
    {
        if (!frame->m_userSEI.payloads)
        {
            frame->m_userSEI.payloads = new x265_sei_payload[numPayloads];
            for (int i = 0; i < numPayloads; i++)
                frame->m_userSEI.payloads[i].payload = NULL;
        }
        for (int i = 0; i < numPayloads; i++)
        {
            x265_sei_payload input;
            if ((i == (numPayloads - 1)) && toneMapPayload)
                input = toneMap;
            else if (m_enableNal)
                input = seiMsg;
            else
                input = pic_in->userSEI.payloads[i];

            if (!frame->m_userSEI.payloads[i].payload)
                frame->m_userSEI.payloads[i].payload = new uint8_t[input.payloadSize];
            memcpy(frame->m_userSEI.payloads[i].payload, input.payload, input.payloadSize);
            frame->m_userSEI.payloads[i].payloadSize = input.payloadSize;
            frame->m_userSEI.payloads[i].payloadType = input.payloadType;
        }

The internal frames are reused, so frame might not be clean; in particular frame->m_userSEI might already be set. And in this case the above code simply presumes that this array is already big enough. Which is just not true (in particular not in this case).

A few lines below the same error happens again with the payload arrays.

[Brad Hards]

Ack. I think the issue is still there, but agree this is not the right repro.