[Regression] VP3 Theora decoding crash with threads >= 3

[Mika Fischer]

Summary of the bug:
Crash in vp3 decoder when usign thread_count != 1

How to reproduce:

#include <assert.h>
extern "C" {
#include <libavcodec/avcodec.h>
#include <libavcodec/packet.h>
#include <libavformat/avformat.h>
#include <libavutil/avutil.h>
#include <libavutil/frame.h>
}

static void check(int64_t ret, int64_t expected) {
    assert(ret == expected);
}

int main() {
    AVFormatContext* ic = NULL;
    check(avformat_open_input(&ic, "blackwhite_yuv444p.ogv", NULL, NULL), 0);
    check(avformat_find_stream_info(ic, NULL), 0);
    check(ic->nb_streams, 1);
    AVCodecContext* avctx = avcodec_alloc_context3(avcodec_find_decoder(ic->streams[0]->codecpar->codec_id));
    assert(avctx);
    avctx->thread_count = 0;
    check(avcodec_parameters_to_context(avctx, ic->streams[0]->codecpar), 0);
    check(avcodec_open2(avctx, NULL, NULL), 0);

    AVPacket *pkt = av_packet_alloc(), *pkt2 = av_packet_alloc();
    check(av_read_frame(ic, pkt), 0);
    check(av_read_frame(ic, pkt2), AVERROR_EOF);

    AVFrame* frame = av_frame_alloc();

    avcodec_flush_buffers(avctx);
    check(avcodec_send_packet(avctx, pkt), 0);
    check(avcodec_send_packet(avctx, nullptr), 0);
    check(avcodec_receive_frame(avctx, frame), 0);
    check(avcodec_receive_frame(avctx, frame), AVERROR_EOF);

    avcodec_flush_buffers(avctx);
    check(avcodec_send_packet(avctx, pkt), 0);
    check(avcodec_send_packet(avctx, nullptr), 0);
    check(avcodec_receive_frame(avctx, frame), 0);
    check(avcodec_receive_frame(avctx, frame), AVERROR_EOF);
}

blackwhite_yuv444p.ogv is from the chromium test suite: ​https://github.com/chromium/chromium/raw/refs/heads/main/media/test/data/blackwhite_yuv444p.ogv

avcodec_flush_buffers replaces the s->coeff_vlc with a nullptr:

#0  ff_refstruct_unref (objp=0x55555558a8a0) at libavcodec/refstruct.c:130
#1  0x00007ffff70995fb in ff_refstruct_replace (dstp=0x55555558a8a0, src=0x0) at libavcodec/refstruct.c:167
#2  0x00007ffff7267ad4 in vp3_update_thread_context (dst=0x5555556cb680, src=0x555555796d00) at libavcodec/vp3.c:2530
#3  0x00007ffff7059335 in update_context_from_thread (dst=0x5555556cb680, src=0x555555796d00, for_user=0) at libavcodec/pthread_frame.c:420
#4  0x00007ffff705afa1 in ff_thread_flush (avctx=0x55555555b100) at libavcodec/pthread_frame.c:994
#5  0x00007ffff6967a4c in avcodec_flush_buffers (avctx=0x55555555b100) at libavcodec/avcodec.c:392
#6  0x000055555555576c in main ()

Subsequent decoding then accesses this null pointer:

Thread 2 "av:theora:df0" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff4ffb640 (LWP 3736249)]
unpack_dct_coeffs (s=0x5555555834c0, gb=0x7ffff4ffaca0) at libavcodec/vp3.c:1319
1319        residual_eob_run = unpack_vlcs(s, gb, coeff_vlc[dc_y_table], 0,
(gdb) bt
#0  unpack_dct_coeffs (s=0x5555555834c0, gb=0x7ffff4ffaca0) at libavcodec/vp3.c:1319
#1  0x00007ffff7268769 in vp3_decode_frame (avctx=0x5555556cb680, frame=0x7fffe8021ac0, got_frame=0x7ffff4ffad00, avpkt=0x5555556cbc40) at libavcodec/vp3.c:2767
#2  0x00007ffff6abb42b in decode_simple_internal (avctx=0x5555556cb680, frame=0x7fffe8021ac0, discarded_samples=0x7ffff4ffad70) at libavcodec/decode.c:442
#3  0x00007ffff6abb99c in decode_simple_receive_frame (avctx=0x5555556cb680, frame=0x7fffe8021ac0) at libavcodec/decode.c:612
#4  0x00007ffff6abbb4b in ff_decode_receive_frame_internal (avctx=0x5555556cb680, frame=0x7fffe8021ac0) at libavcodec/decode.c:648
#5  0x00007ffff7058d50 in frame_worker_thread (arg=0x55555555eb00) at libavcodec/pthread_frame.c:295
#6  0x00007ffff51bdac3 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
#7  0x00007ffff524f850 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

The crash happens in 7.1 and master

[mkver]

The crash can also be reproduced with a simple

ffmpeg -stream_loop 1 -threads <anything > 2> -i blackwhite_yuv444p.ogv -f null -

-threads 2 does not exhibit the issue.

[mkver]

Regression since 5acbdd2264d3b90dc11369f9e031e762f260882e (I initially expected 6c7a344b65cb7476d1575cb1504e3a53bcbc83e7).

[MasterQuestionable]

͏    @ 2024-08-12T12:42:20Z
͏    lavc: convert frame threading to the receive_frame() pattern
​https://github.com/FFmpeg/FFmpeg/commit/5acbdd2264d3b90dc11369f9e031e762f260882e
͏    “Reorganize the code such that the frame threading code does not call the decoders directly:
͏    But instead calls back into the generic decoding code.

͏    This avoids duplicating the logic that wraps the decoder invocation.
͏    And allows "receive_frame"-based decoders to use frame threading.”

͏    ​https://github.com/chromium/chromium/raw/main/media/test/data/blackwhite_yuv444p.ogv
͏    (4,921 B; Ogg: VP3 Theora video, 0.04 s, 25 FPS, 240x240, YUV 4:4:4, 1,520 B; ... 3,401 B)

͏    ffmpeg -v trace -nostdin -nostats -threads 3 -stream_loop 1 -i "blackwhite_yuv444p.ogv" -c copy -f null -
͏    ; where "-threads" >= 3.

[Michael Niedermayer]

Replying to MasterQuestionable:

If this is a regression and a crash, why do you remove the "regression" and "crash" keywords ?

[MasterQuestionable]

͏    That both present in title: probably no point to repeat.
͏    Keywords are supposed to provide hint:
͏    That redundancy tends to reduce.

͏    More verbosity = Less readability
͏    ; generally true.
͏    (though do caveat over-reduction)

͏    Probably not many would search specifically only for the Keywords field.
͏    (and which is nevertheless not well defined)
͏    Thus the practice shouldn't really cause any accessibility degrade.

[Marton Balint]

We use the regression keyword to track regressions, DO NOT remove it for tickets which are confirmed regressions with a known regression commit hash.

[Michael Niedermayer]

Replying to Marton Balint:

We use the regression keyword to track regressions, DO NOT remove it for tickets which are confirmed regressions with a known regression commit hash.

Yes, exactly, regressions should have "regression" as a keyword, it makes search simpler

[MasterQuestionable]

͏    Maybe the proper search should test for both Summary and Keywords?
͏    However I did note the current seemingly inefficiency of the Trac search system:
͏    Lacking proper interface of representing the same test on multiple fields.

͏    I'll change the future handling then.
͏    (generally tagging regression in both Summary, Keywords)

͏    Pardon for the bother.
͏    And thanks for hinting.

[mkver]

Fixed in 90551b7d80e39c2fcde67fc65e3623bbef12590c.