On Sun, Dec 17, 2023 at 04:38:02PM +0100, Theo Buehler wrote: > > mpv crashes after seeking forward(->) any .opus file on obsd current > > Yes, any seek crashes mpv and mplayer. It's a bug in ffmpeg's seek code > in libavformat and the trace always looks the same: > > #0 ff_seek_frame_binary (s=0xfa08e08a800, stream_index=0, > target_ts=<optimized out>, flags=1) > at src/libavformat/utils.c:2190 > #1 0x00000fa126e2475c in ogg_read_seek (s=0xfa08e08a800, stream_index=0, > timestamp=303408, > flags=1) at src/libavformat/oggdec.c:954 > #2 0x00000fa126e8c942 in seek_frame_internal (s=0xfa08e08a800, > stream_index=0, timestamp=303408, > flags=1) at src/libavformat/utils.c:2472 > #3 av_seek_frame (s=0xfa08e08a800, stream_index=<optimized out>, > timestamp=<optimized out>, > flags=1) at src/libavformat/utils.c:2504 > > It's been present since the switch to llvm 16, presumably some more > aggressive optimization around undefined behavior. I haven't spotted > the precise issue, but the below diff works around the crash:
This seems to come from the use of the av_uninit() macro: # define av_uninit(x) x=x "av_uninit() to suppress false uninitialized warnings from gcc without deoptimizing code. https://github.com/FFmpeg/FFmpeg/commit/51066987cf05a7cad567e965fa637e28f9d902c5 This seems a dubious idea to begin with, even 15 years ago, but with the aggressive optimizations of modern compilers it's downright dangerous. int64_t av_uninit(pos_min), av_uninit(pos_max), pos, pos_limit; ... index = av_index_search_timestamp(st, target_ts, flags & ~AVSEEK_FLAG_BACKWARD); av_assert0(index < st->nb_index_entries); if (index >= 0) { e = &st->index_entries[index]; av_assert1(e->timestamp >= target_ts); pos_max = e->pos; The crash happens in the above line, which is the only spot initializing pos_max. ... } pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit, ts_min, ts_max, flags, &ts, avif->read_timestamp); But here pos_max is passed by value to ff_gen_search. Passing by value implies reading the value, which implies the value must be well-defined which in turn means that index >= 0 always. Or something like that. In any case, the compiler elides the index >= 0 check in unmodified code and leaves it there when disabling optimization, or alternatively when initializing pos_max: --- libavformat/utils.c.orig +++ libavformat/utils.c @@ -2146,7 +2146,7 @@ int ff_seek_frame_binary(AVFormatContext *s, int strea int64_t target_ts, int flags) { const AVInputFormat *avif = s->iformat; - int64_t av_uninit(pos_min), av_uninit(pos_max), pos, pos_limit; + int64_t av_uninit(pos_min), pos_max = 0, pos, pos_limit; int64_t ts_min, ts_max, ts; int index; int64_t ret;
