First segment corrupted when using segment & ogg muxers - memory corruption?

[Teodor Woźniak]

I am trying to output segmented .ogg using segment muxer.

If segment_time and segment_atclocktime command line options are specified (value doesn't matter), the first segment is corrupted. ffprobe says:
[ogg @ 0x...] Codec not found

What does work:
-f segment -segment_time 1 'segment%d.ogg'

What causes corruption:
-f segment -segment_time 1 -segment_atclocktime 0 -strftime 0 'segment%d.ogg'
-f segment -segment_time 10 -segment_atclocktime 1 -strftime 1 '%Y-%m-%d_%H-%M-%S.ogg'
The -strftime 1 variant was tested with -re

First segment is fine when using webm container.

It is regression since 4.2.4.

How to reproduce:

ffmpeg -f lavfi -i 'anoisesrc=color=pink[L];anoisesrc=color=pink[R];[L][R]amerge=2' -c:a libvorbis -b:a 224k -f segment -segment_time 1 -segment_atclocktime 0 -strftime 0 -t 2 'segment%d.ogg'
ffprobe segment0.ogg

Git bisect returned a commit which looks unrelated. I'm puzzled.

Tail of bisect output:

b09fb030c15fea2a1cbddf0074c498a415f3fed2 is the first new commit
commit b09fb030c15fea2a1cbddf0074c498a415f3fed2
Author: Fei Wang <fei.w.wang@intel.com>
Date:   Wed Apr 22 13:23:01 2020 +0800

    lavu/pix_fmt: add new pixel format x2rgb10

    The format is packed RGB with each channel 10 bits available and
    include 2 bits unused.

    Signed-off-by: Fei Wang <fei.w.wang@intel.com>

 libavutil/pixdesc.c                     | 24 ++++++++++++++++++++++++
 libavutil/pixfmt.h                      |  3 +++
 libavutil/version.h                     |  2 +-
 tests/ref/fate/filter-pixdesc-x2rgb10le |  1 +
 tests/ref/fate/sws-pixdesc-query        | 11 +++++++++++
 5 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 tests/ref/fate/filter-pixdesc-x2rgb10le
bisect run success

$ git bisect log

git bisect start
# old: [f9f95ceebfbd7b7f43c1b7ad34e25d366e6e2d2b] Changelog: update
git bisect old f9f95ceebfbd7b7f43c1b7ad34e25d366e6e2d2b
# new: [7b1ed4b53a4b32865dd9990f555929e803f64b64] avcodec/proresdec2: let long name match one from codec_desc.c
git bisect new 7b1ed4b53a4b32865dd9990f555929e803f64b64
# old: [22db337a40275599eb8883b40bc03ecfc272ca1a] Bump minor versions to separate 4.2 from master
git bisect old 22db337a40275599eb8883b40bc03ecfc272ca1a
# old: [53a485cd374edff1f0701310af47bf0fd3dd3c0e] avfilter/af_afftfilt: fix memory leaks on error
git bisect old 53a485cd374edff1f0701310af47bf0fd3dd3c0e
# old: [99c58e49e84e055d0a2b8f0372dfa3dbe815819b] avformat/matroskaenc: Clean up mkv_write_stereo_mode()
git bisect old 99c58e49e84e055d0a2b8f0372dfa3dbe815819b
# new: [638ef5f75896e60dc437d0955ced3cb1901a5e25] avformat/smacker: Don't read only one byte at a time
git bisect new 638ef5f75896e60dc437d0955ced3cb1901a5e25
# old: [7aa7d68971e48f6bbf729a6feb318a17010d410f] AVFormatContext: switch to child_class_iterate()
git bisect old 7aa7d68971e48f6bbf729a6feb318a17010d410f
# new: [e3b5897fe3f1b76b4d46bab452cd35a7f308a93a] avcodec/libx265: Fix integer overflow in computation of max and avg bitrate
git bisect new e3b5897fe3f1b76b4d46bab452cd35a7f308a93a
# new: [bd0f81526d3f4c23ecd0a399829103be2445c011] avcodec/pixlet: Fix log(0) check
git bisect new bd0f81526d3f4c23ecd0a399829103be2445c011
# new: [61454bb6fff8f77ad51091b76835c41c6c8ec9d8] avcodec/apedec: add FF_CODEC_CAP_INIT_CLEANUP
git bisect new 61454bb6fff8f77ad51091b76835c41c6c8ec9d8
# old: [38737b3d4e03e2a089083e38cd1fd6f9b4c3ddfd] mailmap: add entry for myself
git bisect old 38737b3d4e03e2a089083e38cd1fd6f9b4c3ddfd
# new: [49ba60fed04d7011c36bae378445ba93ccf983c2] avcodec/cbs: Allocate more CodedBitstreamUnit at once in cbs_insert_unit()
git bisect new 49ba60fed04d7011c36bae378445ba93ccf983c2
# new: [fd54add89c4fed77a30ead2c08103be580881951] doc/APIchanges: add new AV_PIX_FMT_X2RGB10
git bisect new fd54add89c4fed77a30ead2c08103be580881951
# new: [c721b450141d6bbe1e977212a0bcb70118965c34] swscale: Add swscale input/output support for X2RGB10LE
git bisect new c721b450141d6bbe1e977212a0bcb70118965c34
# new: [b09fb030c15fea2a1cbddf0074c498a415f3fed2] lavu/pix_fmt: add new pixel format x2rgb10
git bisect new b09fb030c15fea2a1cbddf0074c498a415f3fed2
# first new commit: [b09fb030c15fea2a1cbddf0074c498a415f3fed2] lavu/pix_fmt: add new pixel format x2rgb10

I attach encoding log, decoding log and bisect helper scripts.
After saving helper scripts in current directory, my tests can be replicated using:

git clone https://git.ffmpeg.org/ffmpeg.git
cd ffmpeg
git bisect start
git bisect old n4.2.4
git bisect new master
git bisect run ../trigger

[Teodor Woźniak]

corrupted segment decoding log

[Teodor Woźniak]

bisect helper script

[Teodor Woźniak]

build helper script

[Teodor Woźniak]

Looks like use-after-free. Attached valgrind logs. Increasing priority because it may have security implications (I'm a bit paranoid but who knows)

[Carl Eugen Hoyos]

Looks like a regression since 3c5a53cdfa099bba8bd951f95b85727b4b3b5d68

[mkver]

In seg_write_header() the segment muxer changes the AVCodecParameters of the stream's used for its internal ogg-muxer after having initialized the ogg muxer; but the ogg muxer has analysed the extradata during its init function and (in case of vorbis and theora) set a pointer pointing into the extradata; this pointer is now dangling. According to the documentation, the AVCodecParameters may be set by the caller (in this case the segment muxer) before calling avformat_write_header(), so that the ogg muxer is violating API as documented (and the way to remedy this is to remove the init function from the ogg muxer and do what is done in ogg_init() as part of ogg_write_header() (ogg_init() can of course still exist as a separate function, but not as init function of the various muxers; instead it should be called from ogg_write_header())).
But on the other hand, the documented behaviour is actually nonsense: If one is allowed to change the AVCodecParameters after avformat_init_output(), one were allowed to change e.g. the codec ID or even the codec type. It meant that basically all checks currently performed in the init functions were premature (ergo invalid), because the caller would be allowed to change the parameters set lateron. For the record, FATE passes when one removes the (btw unchecked) call to avcodec_parameters_copy() in seg_write_header(). This should not break anything as long as the caller doesn't modify the AVCodecParameters between avformat_init_output() and avformat_write_header().
This call has been added in 8e6478b723affe4d44f94d34b98e0c47f6a0b411; at that time, the header was not always written immediately with avformat_write_header(). Depending upon flags (namely the autobsf flag) it was written when the first packet was written, giving the segment muxer time to intercept the packets and extract extradata from them to be used when writing the child muxer's header. Today, the whole code for delaying writing the header is gone, so this serves no useful purpose any more.

[mkver]

Fixed in 92c8b79b5acc06ec608b4c5a2b1ff428dfa1a810.