diff mbox series

[FFmpeg-devel,03/14] pthread_frame: merge the functionality for normal decoder init and init_thread_copy

Message ID 20200327125747.13460-3-anton@khirnov.net
State Accepted
Headers show
Series [FFmpeg-devel,01/14] mpeg4videodec: do not copy a range of fields at once
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Anton Khirnov March 27, 2020, 12:57 p.m. UTC
The current design, where
- proper init is called for the first per-thread context
- first thread's private data is copied into private data for all the
  other threads
- a "fixup" function is called for all the other threads to e.g.
  allocate dynamically allocated data
is very fragile and hard to follow, so it is abandoned. Instead, the
same init function is used to init each per-thread context. Where
necessary, AVCodecInternal.is_copy can be used to differentiate between
the first thread and the other ones (e.g. for decoding the extradata
just once).
---
 doc/multithreading.txt     |  3 ---
 libavcodec/aic.c           |  1 -
 libavcodec/alac.c          | 10 ---------
 libavcodec/avcodec.h       |  6 -----
 libavcodec/cfhd.c          |  6 ++---
 libavcodec/cllc.c          | 14 ------------
 libavcodec/dnxhddec.c      | 16 --------------
 libavcodec/exr.c           | 15 -------------
 libavcodec/ffv1dec.c       | 29 ------------------------
 libavcodec/flacdec.c       | 14 ------------
 libavcodec/h264dec.c       | 38 ++++++++------------------------
 libavcodec/hevcdec.c       | 29 ++++++------------------
 libavcodec/hqx.c           |  3 ---
 libavcodec/huffyuvdec.c    | 32 ---------------------------
 libavcodec/lagarith.c      | 11 ----------
 libavcodec/lcldec.c        |  9 --------
 libavcodec/magicyuv.c      | 16 --------------
 libavcodec/mdec.c          | 12 ----------
 libavcodec/mimic.c         | 22 +------------------
 libavcodec/mpeg4videodec.c | 10 ++++-----
 libavcodec/pixlet.c        | 16 --------------
 libavcodec/pngdec.c        |  6 +----
 libavcodec/proresdec2.c    | 12 ----------
 libavcodec/pthread_frame.c | 31 +++++++++++++++-----------
 libavcodec/rv30.c          |  1 -
 libavcodec/rv34.c          | 28 ------------------------
 libavcodec/rv34.h          |  1 -
 libavcodec/rv40.c          |  1 -
 libavcodec/sheervideo.c    | 14 ------------
 libavcodec/takdec.c        |  8 -------
 libavcodec/tiff.c          |  1 -
 libavcodec/tta.c           |  8 -------
 libavcodec/vble.c          |  1 -
 libavcodec/vp3.c           | 45 --------------------------------------
 libavcodec/vp8.c           | 16 --------------
 libavcodec/vp9.c           |  6 -----
 libavcodec/wavpack.c       | 15 -------------
 libavcodec/ylc.c           | 19 ----------------
 38 files changed, 42 insertions(+), 483 deletions(-)

Comments

David Bryant March 27, 2020, 10:51 p.m. UTC | #1
On 3/27/20 5:57 AM, Anton Khirnov wrote:
> The current design, where
> - proper init is called for the first per-thread context
> - first thread's private data is copied into private data for all the
>   other threads
> - a "fixup" function is called for all the other threads to e.g.
>   allocate dynamically allocated data
> is very fragile and hard to follow, so it is abandoned. Instead, the
> same init function is used to init each per-thread context. Where
> necessary, AVCodecInternal.is_copy can be used to differentiate between
> the first thread and the other ones (e.g. for decoding the extradata
> just once).
> ---

I'm not sure I fully understand this change. You mention that AVCodecInternal.is_copy can be used where different treatment
might be necessary for subsequent threads, and I see that done in a couple places, but in most cases you have simply deleted the
init_thread_copy() function even when it's not at all obvious (to me anyway) that that won't break the codec.

Specifically this will break WavPack because now it will be allocating a new dsdctx for every thread instead of sharing one
between all threads. I am happy to fix and test this case once the patch goes in, but is the intent of this patch that the
respective authors will find and fix all the breakages? Or did I just happen to catch the one case you missed?

Cheers!

-David
Anton Khirnov March 28, 2020, 1:23 p.m. UTC | #2
Quoting David Bryant (2020-03-27 23:51:19)
> On 3/27/20 5:57 AM, Anton Khirnov wrote:
> > The current design, where
> > - proper init is called for the first per-thread context
> > - first thread's private data is copied into private data for all the
> >   other threads
> > - a "fixup" function is called for all the other threads to e.g.
> >   allocate dynamically allocated data
> > is very fragile and hard to follow, so it is abandoned. Instead, the
> > same init function is used to init each per-thread context. Where
> > necessary, AVCodecInternal.is_copy can be used to differentiate between
> > the first thread and the other ones (e.g. for decoding the extradata
> > just once).
> > ---
> 
> I'm not sure I fully understand this change. You mention that AVCodecInternal.is_copy can be used where different treatment
> might be necessary for subsequent threads, and I see that done in a couple places, but in most cases you have simply deleted the
> init_thread_copy() function even when it's not at all obvious (to me anyway) that that won't break the codec.

In most cases, just deleting init_thread_copy() is the right thing to
do. E.g. all decoders that do not implement update_thread_context() have
to be intra-only, so every frame thread is effectively a completely
standalone decoder. And in most of the more complex decoders (like h264)
the important parameters are dynamically changeable during decoding, so
not that much is done in decoder init beyond allocating some stuff that
does not depend on the bistream properties.

My intent is for each frame-thread worker to be treated inasmuch as
possible as a standalone decoder, and where it has to share data with
other threads to make this sharing explicit (rather than implicit as is
the case now).

> 
> Specifically this will break WavPack because now it will be allocating a new dsdctx for every thread instead of sharing one
> between all threads. I am happy to fix and test this case once the patch goes in, but is the intent of this patch that the
> respective authors will find and fix all the breakages? Or did I just happen to catch the one case you missed?

I certainly intended to convert the decoders correctly myself,
apparently I didn't pay enough attention to the recent wavpack changes.
Hopefully the other conversions are correct, but I will go through the
changes again to check.

Looking at wavpack, the code looks suspicious to me. You allocate one
dsdctx per channel at init, but AFAIU the number of channels can change
at each frame.
David Bryant March 28, 2020, 8:22 p.m. UTC | #3
On 3/28/20 6:23 AM, Anton Khirnov wrote:
> Quoting David Bryant (2020-03-27 23:51:19)
>> On 3/27/20 5:57 AM, Anton Khirnov wrote:
>>> The current design, where
>>> - proper init is called for the first per-thread context
>>> - first thread's private data is copied into private data for all the
>>>   other threads
>>> - a "fixup" function is called for all the other threads to e.g.
>>>   allocate dynamically allocated data
>>> is very fragile and hard to follow, so it is abandoned. Instead, the
>>> same init function is used to init each per-thread context. Where
>>> necessary, AVCodecInternal.is_copy can be used to differentiate between
>>> the first thread and the other ones (e.g. for decoding the extradata
>>> just once).
>>> ---
>> I'm not sure I fully understand this change. You mention that AVCodecInternal.is_copy can be used where different treatment
>> might be necessary for subsequent threads, and I see that done in a couple places, but in most cases you have simply deleted the
>> init_thread_copy() function even when it's not at all obvious (to me anyway) that that won't break the codec.
> In most cases, just deleting init_thread_copy() is the right thing to
> do. E.g. all decoders that do not implement update_thread_context() have
> to be intra-only, so every frame thread is effectively a completely
> standalone decoder. And in most of the more complex decoders (like h264)
> the important parameters are dynamically changeable during decoding, so
> not that much is done in decoder init beyond allocating some stuff that
> does not depend on the bistream properties.
>
> My intent is for each frame-thread worker to be treated inasmuch as
> possible as a standalone decoder, and where it has to share data with
> other threads to make this sharing explicit (rather than implicit as is
> the case now).

Yes, this makes sense. The confusing part is when the decode_init() function looks completely different than the
init_thread_copy() function. This is often because the decode_init() function is generating things (tables, etc.) from scratch
and the init_thread_copy() is just copying the necessary things from the original. In cases where the original generation might
be time consuming this might make sense, but usually it's probably just making the code more complex and difficult to follow
(which I believe was your original point).

One possible interim solution for complex cases that break would be to leave the init_thread_copy() function there, but instead
of having it in the AVCodec struct and called from outside (which is no longer possible), it simply gets called first thing from
decode_init() if AVCodecInternal.is_copy is set. That way the architecture is cleaned up now, and the codec won't break and can
be cleaned up when time permits. Just a thought.

>
>> Specifically this will break WavPack because now it will be allocating a new dsdctx for every thread instead of sharing one
>> between all threads. I am happy to fix and test this case once the patch goes in, but is the intent of this patch that the
>> respective authors will find and fix all the breakages? Or did I just happen to catch the one case you missed?
> I certainly intended to convert the decoders correctly myself,
> apparently I didn't pay enough attention to the recent wavpack changes.
> Hopefully the other conversions are correct, but I will go through the
> changes again to check.
>
> Looking at wavpack, the code looks suspicious to me. You allocate one
> dsdctx per channel at init, but AFAIU the number of channels can change
> at each frame.
>
Multichannel WavPack consists of sequences of stereo or mono WavPack blocks that include flags indicating whether they are the
beginning or end of a "frame" or "packet". This allows the number of channels available to be essentially unlimited, however the
total number of channels in a stream may not change. When decoding, if a sequence of blocks generates more or less than the
correct number of channels, this is flagged as an error and overrun is prevented (see libavcodec/wavpack.c, line 1499 and line
1621).

If you are curious, the WavPack format is described in detail here:

https://github.com/dbry/WavPack/blob/master/doc/WavPack5FileFormat.pdf

Thanks!

-David
Anton Khirnov March 31, 2020, 9:47 a.m. UTC | #4
Quoting David Bryant (2020-03-28 21:22:40)
> On 3/28/20 6:23 AM, Anton Khirnov wrote:
> > Quoting David Bryant (2020-03-27 23:51:19)
> >> On 3/27/20 5:57 AM, Anton Khirnov wrote:
> >>> The current design, where
> >>> - proper init is called for the first per-thread context
> >>> - first thread's private data is copied into private data for all the
> >>>   other threads
> >>> - a "fixup" function is called for all the other threads to e.g.
> >>>   allocate dynamically allocated data
> >>> is very fragile and hard to follow, so it is abandoned. Instead, the
> >>> same init function is used to init each per-thread context. Where
> >>> necessary, AVCodecInternal.is_copy can be used to differentiate between
> >>> the first thread and the other ones (e.g. for decoding the extradata
> >>> just once).
> >>> ---
> >> I'm not sure I fully understand this change. You mention that AVCodecInternal.is_copy can be used where different treatment
> >> might be necessary for subsequent threads, and I see that done in a couple places, but in most cases you have simply deleted the
> >> init_thread_copy() function even when it's not at all obvious (to me anyway) that that won't break the codec.
> > In most cases, just deleting init_thread_copy() is the right thing to
> > do. E.g. all decoders that do not implement update_thread_context() have
> > to be intra-only, so every frame thread is effectively a completely
> > standalone decoder. And in most of the more complex decoders (like h264)
> > the important parameters are dynamically changeable during decoding, so
> > not that much is done in decoder init beyond allocating some stuff that
> > does not depend on the bistream properties.
> >
> > My intent is for each frame-thread worker to be treated inasmuch as
> > possible as a standalone decoder, and where it has to share data with
> > other threads to make this sharing explicit (rather than implicit as is
> > the case now).
> 
> Yes, this makes sense. The confusing part is when the decode_init() function looks completely different than the
> init_thread_copy() function. This is often because the decode_init() function is generating things (tables, etc.) from scratch
> and the init_thread_copy() is just copying the necessary things from the original. In cases where the original generation might
> be time consuming this might make sense, but usually it's probably just making the code more complex and difficult to follow
> (which I believe was your original point).

Yes, precisely. I believe for obscure codecs it's better to waste a
little memory when doing threaded decoding, in return for simpler code.
And if people really care about being as memory-efficient as possible
then doing explicit refcounting on allocated buffers is still possible.

> 
> One possible interim solution for complex cases that break would be to leave the init_thread_copy() function there, but instead
> of having it in the AVCodec struct and called from outside (which is no longer possible), it simply gets called first thing from
> decode_init() if AVCodecInternal.is_copy is set. That way the architecture is cleaned up now, and the codec won't break and can
> be cleaned up when time permits. Just a thought.

I don't really see much point in any interim solutions, since no codecs
should break. You found a problem with wavpack, that's going to be fixed
in the next iteration of this patch.
Speaking of which, ideally we should have a FATE test for DSD files so
that I would have found the failure myself when running test.

> 
> >
> >> Specifically this will break WavPack because now it will be allocating a new dsdctx for every thread instead of sharing one
> >> between all threads. I am happy to fix and test this case once the patch goes in, but is the intent of this patch that the
> >> respective authors will find and fix all the breakages? Or did I just happen to catch the one case you missed?
> > I certainly intended to convert the decoders correctly myself,
> > apparently I didn't pay enough attention to the recent wavpack changes.
> > Hopefully the other conversions are correct, but I will go through the
> > changes again to check.
> >
> > Looking at wavpack, the code looks suspicious to me. You allocate one
> > dsdctx per channel at init, but AFAIU the number of channels can change
> > at each frame.
> >
> Multichannel WavPack consists of sequences of stereo or mono WavPack blocks that include flags indicating whether they are the
> beginning or end of a "frame" or "packet". This allows the number of channels available to be essentially unlimited, however the
> total number of channels in a stream may not change. When decoding, if a sequence of blocks generates more or less than the
> correct number of channels, this is flagged as an error and overrun is prevented (see libavcodec/wavpack.c, line 1499 and line
> 1621).

If my reading of the code is correct, line 1499 only checks for overflow
of the channel count read for the current sequence. That channel count
is exported to AVCodeContext in the block starting on line 1464 and
I don't see any checks for whether it is equal to the initial one.
David Bryant April 1, 2020, 9:35 p.m. UTC | #5
On 3/31/20 2:47 AM, Anton Khirnov wrote:
> Quoting David Bryant (2020-03-28 21:22:40)
>> On 3/28/20 6:23 AM, Anton Khirnov wrote:
>>> Quoting David Bryant (2020-03-27 23:51:19)
>>>> On 3/27/20 5:57 AM, Anton Khirnov wrote:
>>>>> The current design, where
>>>>> - proper init is called for the first per-thread context
>>>>> - first thread's private data is copied into private data for all the
>>>>>   other threads
>>>>> - a "fixup" function is called for all the other threads to e.g.
>>>>>   allocate dynamically allocated data
>>>>> is very fragile and hard to follow, so it is abandoned. Instead, the
>>>>> same init function is used to init each per-thread context. Where
>>>>> necessary, AVCodecInternal.is_copy can be used to differentiate between
>>>>> the first thread and the other ones (e.g. for decoding the extradata
>>>>> just once).
>>>>> ---
>>>> I'm not sure I fully understand this change. You mention that AVCodecInternal.is_copy can be used where different treatment
>>>> might be necessary for subsequent threads, and I see that done in a couple places, but in most cases you have simply deleted the
>>>> init_thread_copy() function even when it's not at all obvious (to me anyway) that that won't break the codec.
>>> In most cases, just deleting init_thread_copy() is the right thing to
>>> do. E.g. all decoders that do not implement update_thread_context() have
>>> to be intra-only, so every frame thread is effectively a completely
>>> standalone decoder. And in most of the more complex decoders (like h264)
>>> the important parameters are dynamically changeable during decoding, so
>>> not that much is done in decoder init beyond allocating some stuff that
>>> does not depend on the bistream properties.
>>>
>>> My intent is for each frame-thread worker to be treated inasmuch as
>>> possible as a standalone decoder, and where it has to share data with
>>> other threads to make this sharing explicit (rather than implicit as is
>>> the case now).
>> Yes, this makes sense. The confusing part is when the decode_init() function looks completely different than the
>> init_thread_copy() function. This is often because the decode_init() function is generating things (tables, etc.) from scratch
>> and the init_thread_copy() is just copying the necessary things from the original. In cases where the original generation might
>> be time consuming this might make sense, but usually it's probably just making the code more complex and difficult to follow
>> (which I believe was your original point).
> Yes, precisely. I believe for obscure codecs it's better to waste a
> little memory when doing threaded decoding, in return for simpler code.
> And if people really care about being as memory-efficient as possible
> then doing explicit refcounting on allocated buffers is still possible.
>
>> One possible interim solution for complex cases that break would be to leave the init_thread_copy() function there, but instead
>> of having it in the AVCodec struct and called from outside (which is no longer possible), it simply gets called first thing from
>> decode_init() if AVCodecInternal.is_copy is set. That way the architecture is cleaned up now, and the codec won't break and can
>> be cleaned up when time permits. Just a thought.
> I don't really see much point in any interim solutions, since no codecs
> should break. You found a problem with wavpack, that's going to be fixed
> in the next iteration of this patch.
> Speaking of which, ideally we should have a FATE test for DSD files so
> that I would have found the failure myself when running test.

I have created a WavPack DSD test file that exercises all three of the DSD modes (fast, high, and copy). I have also verified
that it would have caught this change, although I had to specify fate multithreading once I discovered it wasn't the default. If
someone can add this to the fate suite (in the wavpack/lossless folder) then I can supply a patch to add the test:

file: http://www.wavpack.com/dsd.wv (MD5: 74b2181f3e9829d9a5b98edd037984ac)
decoded MD5 (f32le): a3a88bba95f809025dce01ecb9064091

>>>> Specifically this will break WavPack because now it will be allocating a new dsdctx for every thread instead of sharing one
>>>> between all threads. I am happy to fix and test this case once the patch goes in, but is the intent of this patch that the
>>>> respective authors will find and fix all the breakages? Or did I just happen to catch the one case you missed?
>>> I certainly intended to convert the decoders correctly myself,
>>> apparently I didn't pay enough attention to the recent wavpack changes.
>>> Hopefully the other conversions are correct, but I will go through the
>>> changes again to check.
>>>
>>> Looking at wavpack, the code looks suspicious to me. You allocate one
>>> dsdctx per channel at init, but AFAIU the number of channels can change
>>> at each frame.
>>>
>> Multichannel WavPack consists of sequences of stereo or mono WavPack blocks that include flags indicating whether they are the
>> beginning or end of a "frame" or "packet". This allows the number of channels available to be essentially unlimited, however the
>> total number of channels in a stream may not change. When decoding, if a sequence of blocks generates more or less than the
>> correct number of channels, this is flagged as an error and overrun is prevented (see libavcodec/wavpack.c, line 1499 and line
>> 1621).
> If my reading of the code is correct, line 1499 only checks for overflow
> of the channel count read for the current sequence. That channel count
> is exported to AVCodeContext in the block starting on line 1464 and
> I don't see any checks for whether it is equal to the initial one.
>
Full disclosure: I did not write the WavPack decoder. I understand _how_ it works, but I don't necessarily understand _why_ it
was written the way it was due to gaps in my knowledge of the FFmpeg internals, and there are certainly things in there that
don't look quite right to me.

That said, I have tested it thoroughly and it handles everything I throw at it, and I have extended it to handle the DSD mode
without anything unexpected happening. And of course it seems to handle extensive fuzzing, and the recent error was related to
my DSD change, suggesting that the core it pretty robust. My inclination has been to not "fix" anything that wasn't broken.

Yes, that channel count stuff is a little difficult to follow. I could experiment with streams that abruptly change channel
configuration, but again I'm hesitant to change code that's worked so well so long.

Kind regards,

David
Anton Khirnov April 2, 2020, 9:13 a.m. UTC | #6
Quoting David Bryant (2020-04-01 23:35:13)
> On 3/31/20 2:47 AM, Anton Khirnov wrote:
> >>> Looking at wavpack, the code looks suspicious to me. You allocate one
> >>> dsdctx per channel at init, but AFAIU the number of channels can change
> >>> at each frame.
> >>>
> >> Multichannel WavPack consists of sequences of stereo or mono WavPack blocks that include flags indicating whether they are the
> >> beginning or end of a "frame" or "packet". This allows the number of channels available to be essentially unlimited, however the
> >> total number of channels in a stream may not change. When decoding, if a sequence of blocks generates more or less than the
> >> correct number of channels, this is flagged as an error and overrun is prevented (see libavcodec/wavpack.c, line 1499 and line
> >> 1621).
> > If my reading of the code is correct, line 1499 only checks for overflow
> > of the channel count read for the current sequence. That channel count
> > is exported to AVCodeContext in the block starting on line 1464 and
> > I don't see any checks for whether it is equal to the initial one.
> >
> Full disclosure: I did not write the WavPack decoder. I understand _how_ it works, but I don't necessarily understand _why_ it
> was written the way it was due to gaps in my knowledge of the FFmpeg internals, and there are certainly things in there that
> don't look quite right to me.
> 
> That said, I have tested it thoroughly and it handles everything I throw at it, and I have extended it to handle the DSD mode
> without anything unexpected happening. And of course it seems to handle extensive fuzzing, and the recent error was related to
> my DSD change, suggesting that the core it pretty robust. My inclination has been to not "fix" anything that wasn't broken.
> 
> Yes, that channel count stuff is a little difficult to follow. I could experiment with streams that abruptly change channel
> configuration, but again I'm hesitant to change code that's worked so well so long.

I can easily get the decoder to crash even in single-threaded mode. Just
make the demuxer always say the stream is mono, then try decoding your
DSD sample.

Also, I looked at the specification PDF you linked in the previous email
and I see nothing in it that explicitly forbids the channel count from
changing from one "superblock" to the next. It seems to me like you
could just concatenate wavpack files with different channel counts and
get a valid wavpack file.
David Bryant April 3, 2020, 5:14 a.m. UTC | #7
On 4/2/20 2:13 AM, Anton Khirnov wrote:
> Quoting David Bryant (2020-04-01 23:35:13)
>> On 3/31/20 2:47 AM, Anton Khirnov wrote:
>>>>> Looking at wavpack, the code looks suspicious to me. You allocate one
>>>>> dsdctx per channel at init, but AFAIU the number of channels can change
>>>>> at each frame.
>>>>>
>>>> Multichannel WavPack consists of sequences of stereo or mono WavPack blocks that include flags indicating whether they are the
>>>> beginning or end of a "frame" or "packet". This allows the number of channels available to be essentially unlimited, however the
>>>> total number of channels in a stream may not change. When decoding, if a sequence of blocks generates more or less than the
>>>> correct number of channels, this is flagged as an error and overrun is prevented (see libavcodec/wavpack.c, line 1499 and line
>>>> 1621).
>>> If my reading of the code is correct, line 1499 only checks for overflow
>>> of the channel count read for the current sequence. That channel count
>>> is exported to AVCodeContext in the block starting on line 1464 and
>>> I don't see any checks for whether it is equal to the initial one.
>>>
>> Full disclosure: I did not write the WavPack decoder. I understand _how_ it works, but I don't necessarily understand _why_ it
>> was written the way it was due to gaps in my knowledge of the FFmpeg internals, and there are certainly things in there that
>> don't look quite right to me.
>>
>> That said, I have tested it thoroughly and it handles everything I throw at it, and I have extended it to handle the DSD mode
>> without anything unexpected happening. And of course it seems to handle extensive fuzzing, and the recent error was related to
>> my DSD change, suggesting that the core it pretty robust. My inclination has been to not "fix" anything that wasn't broken.
>>
>> Yes, that channel count stuff is a little difficult to follow. I could experiment with streams that abruptly change channel
>> configuration, but again I'm hesitant to change code that's worked so well so long.
> I can easily get the decoder to crash even in single-threaded mode. Just
> make the demuxer always say the stream is mono, then try decoding your
> DSD sample.

Okay, I reproduced that. However, I'm not sure it's really that serious because the demuxer and the decoder use basically the
same logic to determine that number of channels, so it would be tricky to craft a file that caused the demuxer to calculate 1
channel and the decoder to reach a different result, which is probably why the fuzzers have not been able to do so.

That said, it would not be hard to have the decoder save away the initial channel count from avctx, never overwrite it, and
verify that all superblocks match that count. I don't know off hand whether that would break anything though. Is it guaranteed
that avctx->channels is correct during the call to decode_init(), even if it's being demuxed from Matroska, for example? If so,
why is the WavPack decoder _ever_ writing to the avctx channel count?

>
> Also, I looked at the specification PDF you linked in the previous email
> and I see nothing in it that explicitly forbids the channel count from
> changing from one "superblock" to the next. It seems to me like you
> could just concatenate wavpack files with different channel counts and
> get a valid wavpack file.
>
You're right, I could not find that being explicitly forbidden. Nevertheless, it _is_ forbidden for any stream parameters to
change (e.g. data format, sampling rate, channel count or mapping) and both libwavpack and the FFmpeg demuxer enforce this (see
libavformat/wvdec.c starting at line 211), or at least gracefully fail without crashing.

The reason it's never mentioned is probably just because it never occurred to me. None of the source file formats for WavPack
(e.g., wav, caf, dsdiff) allow format changes and neither do any of the other lossless compressors I am aware of. I can't
imagine how that would work, and extensive fuzzing has ensured that pathological files like you suggest are unlikely to be the
source of exploits.

Kind regards,

David
Anton Khirnov April 3, 2020, 6:32 a.m. UTC | #8
Quoting David Bryant (2020-04-03 07:14:21)
> On 4/2/20 2:13 AM, Anton Khirnov wrote:
> > Quoting David Bryant (2020-04-01 23:35:13)
> >> On 3/31/20 2:47 AM, Anton Khirnov wrote:
> >>>>> Looking at wavpack, the code looks suspicious to me. You allocate one
> >>>>> dsdctx per channel at init, but AFAIU the number of channels can change
> >>>>> at each frame.
> >>>>>
> >>>> Multichannel WavPack consists of sequences of stereo or mono WavPack blocks that include flags indicating whether they are the
> >>>> beginning or end of a "frame" or "packet". This allows the number of channels available to be essentially unlimited, however the
> >>>> total number of channels in a stream may not change. When decoding, if a sequence of blocks generates more or less than the
> >>>> correct number of channels, this is flagged as an error and overrun is prevented (see libavcodec/wavpack.c, line 1499 and line
> >>>> 1621).
> >>> If my reading of the code is correct, line 1499 only checks for overflow
> >>> of the channel count read for the current sequence. That channel count
> >>> is exported to AVCodeContext in the block starting on line 1464 and
> >>> I don't see any checks for whether it is equal to the initial one.
> >>>
> >> Full disclosure: I did not write the WavPack decoder. I understand _how_ it works, but I don't necessarily understand _why_ it
> >> was written the way it was due to gaps in my knowledge of the FFmpeg internals, and there are certainly things in there that
> >> don't look quite right to me.
> >>
> >> That said, I have tested it thoroughly and it handles everything I throw at it, and I have extended it to handle the DSD mode
> >> without anything unexpected happening. And of course it seems to handle extensive fuzzing, and the recent error was related to
> >> my DSD change, suggesting that the core it pretty robust. My inclination has been to not "fix" anything that wasn't broken.
> >>
> >> Yes, that channel count stuff is a little difficult to follow. I could experiment with streams that abruptly change channel
> >> configuration, but again I'm hesitant to change code that's worked so well so long.
> > I can easily get the decoder to crash even in single-threaded mode. Just
> > make the demuxer always say the stream is mono, then try decoding your
> > DSD sample.
> 
> Okay, I reproduced that. However, I'm not sure it's really that serious because the demuxer and the decoder use basically the
> same logic to determine that number of channels, so it would be tricky to craft a file that caused the demuxer to calculate 1
> channel and the decoder to reach a different result, which is probably why the fuzzers have not been able to do so.

I disagree - this is just as serious as any other crash on invalid data.

The decoder must not ever crash for arbitrary input data. It cannot
assume that it's always going to be provided valid input or that it's
always used with some specific demuxer. The caller might not be using
libavformat at all, libavcodec should work with any source of data.

Besides, I can reproduce the crash with no code modification necessary
by muxing your sample into Matroska and then hex-editing the file to say
it has only one channel.

> 
> That said, it would not be hard to have the decoder save away the initial channel count from avctx, never overwrite it, and
> verify that all superblocks match that count. I don't know off hand whether that would break anything though. Is it guaranteed
> that avctx->channels is correct during the call to decode_init(), even if it's being demuxed from Matroska, for example? If so,
> why is the WavPack decoder _ever_ writing to the avctx channel count?

The question is what you define as correct. The initial value provided
by the caller is supposed to be the caller's best guess at the correct
(initial) value. It may come from a demuxer, or from the Delphic Oracle.
The decoder doesn't know.

For some codecs, the relevant parameters are not specified in the
bistream, so they have no choice but to use the caller's value. That is
not the case for WavPack, since the channel count IS written in the
bistream. So I think it's best to ignore the caller-provided value and
always use the one we get from the data.

> 
> >
> > Also, I looked at the specification PDF you linked in the previous email
> > and I see nothing in it that explicitly forbids the channel count from
> > changing from one "superblock" to the next. It seems to me like you
> > could just concatenate wavpack files with different channel counts and
> > get a valid wavpack file.
> >
> You're right, I could not find that being explicitly forbidden. Nevertheless, it _is_ forbidden for any stream parameters to
> change (e.g. data format, sampling rate, channel count or mapping) and both libwavpack and the FFmpeg demuxer enforce this (see
> libavformat/wvdec.c starting at line 211), or at least gracefully fail without crashing.

Forbidden by whom? From what I can see:
- sample format: the wv demuxer does not set it at all, the decoder sets
  it in wavpack_decode_frame() from the block header; also, Michael sent
  a patch recently to deal with a wavpack decoder crash related to
  format changes
- sample rate: set by wv demuxer and then checked that it does not
  change; but the caller is not necessarily using the wv demuxer and the
  decoder will happily change it the same way it does with channel count

> 
> The reason it's never mentioned is probably just because it never occurred to me. None of the source file formats for WavPack
> (e.g., wav, caf, dsdiff) allow format changes and neither do any of the other lossless compressors I am aware of.

Correct, I cannot see any other lossless codecs that would allow this.
But then again all these codecs have global headers. So it's not that
they artificially forbid format changes, but the bistream format is not
able to represent them.

That is different for WavPack - it goes out of its way to make its
blocks independent with no global headers. All other codecs that work
like this allow format changes, so it seems natural that WavPack should
as well.

As I said before - it seems to me that you could simply concatenate two
WavPack files and get a valid WavPack file.

> I can't imagine how that would work,

Why not? We have plenty of decoders where the format or channel count
can change whenever.

> and extensive fuzzing has ensured that pathological files like you
> suggest are unlikely to be the source of exploits.

See the crash above. I think fuzzing is very overhyped these days. It's
a useful tool for sure, but one should not rely purely on fuzzing too
much. It may not cover important parts of the input space because of the
way it is set up. Or there could be exploitable codepaths that require
too specific inputs for a random process to find, but a determined
attacker could construct them from reading the code.
David Bryant April 6, 2020, 4:11 a.m. UTC | #9
On 4/2/20 11:32 PM, Anton Khirnov wrote:
> Quoting David Bryant (2020-04-03 07:14:21)
>> On 4/2/20 2:13 AM, Anton Khirnov wrote:
>>> Quoting David Bryant (2020-04-01 23:35:13)
>>>> On 3/31/20 2:47 AM, Anton Khirnov wrote:
>>>>>>> Looking at wavpack, the code looks suspicious to me. You allocate one
>>>>>>> dsdctx per channel at init, but AFAIU the number of channels can change
>>>>>>> at each frame.
>>>>>>>
>>>>>> Multichannel WavPack consists of sequences of stereo or mono WavPack blocks that include flags indicating whether they are the
>>>>>> beginning or end of a "frame" or "packet". This allows the number of channels available to be essentially unlimited, however the
>>>>>> total number of channels in a stream may not change. When decoding, if a sequence of blocks generates more or less than the
>>>>>> correct number of channels, this is flagged as an error and overrun is prevented (see libavcodec/wavpack.c, line 1499 and line
>>>>>> 1621).
>>>>> If my reading of the code is correct, line 1499 only checks for overflow
>>>>> of the channel count read for the current sequence. That channel count
>>>>> is exported to AVCodeContext in the block starting on line 1464 and
>>>>> I don't see any checks for whether it is equal to the initial one.
>>>>>
>>>> Full disclosure: I did not write the WavPack decoder. I understand _how_ it works, but I don't necessarily understand _why_ it
>>>> was written the way it was due to gaps in my knowledge of the FFmpeg internals, and there are certainly things in there that
>>>> don't look quite right to me.
>>>>
>>>> That said, I have tested it thoroughly and it handles everything I throw at it, and I have extended it to handle the DSD mode
>>>> without anything unexpected happening. And of course it seems to handle extensive fuzzing, and the recent error was related to
>>>> my DSD change, suggesting that the core it pretty robust. My inclination has been to not "fix" anything that wasn't broken.
>>>>
>>>> Yes, that channel count stuff is a little difficult to follow. I could experiment with streams that abruptly change channel
>>>> configuration, but again I'm hesitant to change code that's worked so well so long.
>>> I can easily get the decoder to crash even in single-threaded mode. Just
>>> make the demuxer always say the stream is mono, then try decoding your
>>> DSD sample.
>> Okay, I reproduced that. However, I'm not sure it's really that serious because the demuxer and the decoder use basically the
>> same logic to determine that number of channels, so it would be tricky to craft a file that caused the demuxer to calculate 1
>> channel and the decoder to reach a different result, which is probably why the fuzzers have not been able to do so.
> I disagree - this is just as serious as any other crash on invalid data.
>
> The decoder must not ever crash for arbitrary input data. It cannot
> assume that it's always going to be provided valid input or that it's
> always used with some specific demuxer. The caller might not be using
> libavformat at all, libavcodec should work with any source of data.
>
> Besides, I can reproduce the crash with no code modification necessary
> by muxing your sample into Matroska and then hex-editing the file to say
> it has only one channel.

Yes, I have reproduced this crash even without Matroska. I was working on a solution but you beat me to it. Thanks!


>
>> That said, it would not be hard to have the decoder save away the initial channel count from avctx, never overwrite it, and
>> verify that all superblocks match that count. I don't know off hand whether that would break anything though. Is it guaranteed
>> that avctx->channels is correct during the call to decode_init(), even if it's being demuxed from Matroska, for example? If so,
>> why is the WavPack decoder _ever_ writing to the avctx channel count?
> The question is what you define as correct. The initial value provided
> by the caller is supposed to be the caller's best guess at the correct
> (initial) value. It may come from a demuxer, or from the Delphic Oracle.
> The decoder doesn't know.
>
> For some codecs, the relevant parameters are not specified in the
> bistream, so they have no choice but to use the caller's value. That is
> not the case for WavPack, since the channel count IS written in the
> bistream. So I think it's best to ignore the caller-provided value and
> always use the one we get from the data.

Agreed.


>
>>> Also, I looked at the specification PDF you linked in the previous email
>>> and I see nothing in it that explicitly forbids the channel count from
>>> changing from one "superblock" to the next. It seems to me like you
>>> could just concatenate wavpack files with different channel counts and
>>> get a valid wavpack file.
>>>
>> You're right, I could not find that being explicitly forbidden. Nevertheless, it _is_ forbidden for any stream parameters to
>> change (e.g. data format, sampling rate, channel count or mapping) and both libwavpack and the FFmpeg demuxer enforce this (see
>> libavformat/wvdec.c starting at line 211), or at least gracefully fail without crashing.
> Forbidden by whom? From what I can see:
> - sample format: the wv demuxer does not set it at all, the decoder sets
>   it in wavpack_decode_frame() from the block header; also, Michael sent
>   a patch recently to deal with a wavpack decoder crash related to
>   format changes
> - sample rate: set by wv demuxer and then checked that it does not
>   change; but the caller is not necessarily using the wv demuxer and the
>   decoder will happily change it the same way it does with channel count
>
>> The reason it's never mentioned is probably just because it never occurred to me. None of the source file formats for WavPack
>> (e.g., wav, caf, dsdiff) allow format changes and neither do any of the other lossless compressors I am aware of.
> Correct, I cannot see any other lossless codecs that would allow this.
> But then again all these codecs have global headers. So it's not that
> they artificially forbid format changes, but the bistream format is not
> able to represent them.

I disagree with that. It would be trivial to add a new STREAMINFO block to a FLAC file and change the format after that. This is
not specifically prohibited in the FLAC specs, but it's sort of implied when it says that the initial STREAMINFO block has
"information about the whole stream".

WAV files could certainly have multiple <fmt> and <data> pairs in a file, and I could not find any mention of this being
prohibited, but you would be hard pressed to find a program that would deal with that correctly.

And WavPack actually does have a global header of sorts. The first block that contains audio determines the format of the entire
file. I could not find that explicitly stated anywhere, but it is how it works, and changing the format in the middle of the
file is undefined behavior (although it should not crash, obviously).


>
> That is different for WavPack - it goes out of its way to make its
> blocks independent with no global headers. All other codecs that work
> like this allow format changes, so it seems natural that WavPack should
> as well.
>
> As I said before - it seems to me that you could simply concatenate two
> WavPack files and get a valid WavPack file.

Well, you actually can't produce a valid file that way because the WavPack blocks include information required for seeking. But
if you got around that it would still not be a valid file, by convention.

>
>> I can't imagine how that would work,
> Why not? We have plenty of decoders where the format or channel count
> can change whenever.

Well, there are all sorts of reasons why this would not be appropriate for native WavPack files, and what they are used for
today. For example, if the sample rate changed it would be impossible to determine the duration without scanning the whole file.
Also, libwavpack does not have the ability to generate such files nor the APIs that would allow decoding them in any reasonable way.

That said, I am only referring to the native WavPack container file format (.wv and .wvc). If WavPack streams are embedded in
another container (e.g., Matroska) then this is fine (assuming the container format is happy with it) because, as you say, each
WavPack audio block is independent.


>
>> and extensive fuzzing has ensured that pathological files like you
>> suggest are unlikely to be the source of exploits.
> See the crash above. I think fuzzing is very overhyped these days. It's
> a useful tool for sure, but one should not rely purely on fuzzing too
> much. It may not cover important parts of the input space because of the
> way it is set up. Or there could be exploitable codepaths that require
> too specific inputs for a random process to find, but a determined
> attacker could construct them from reading the code.
>
Of course fuzzing cannot find everything, but my experience has been that fuzzing finds things that I would _never_ find by just
looking at the code.

And I was certainly not suggesting that one should just write sloppy code knowing that all the bugs will be found with fuzzing.

-David
Anton Khirnov April 6, 2020, 8:19 a.m. UTC | #10
Quoting David Bryant (2020-04-06 06:11:17)
> On 4/2/20 11:32 PM, Anton Khirnov wrote:
> >
> >>> Also, I looked at the specification PDF you linked in the previous email
> >>> and I see nothing in it that explicitly forbids the channel count from
> >>> changing from one "superblock" to the next. It seems to me like you
> >>> could just concatenate wavpack files with different channel counts and
> >>> get a valid wavpack file.
> >>>
> >> You're right, I could not find that being explicitly forbidden. Nevertheless, it _is_ forbidden for any stream parameters to
> >> change (e.g. data format, sampling rate, channel count or mapping) and both libwavpack and the FFmpeg demuxer enforce this (see
> >> libavformat/wvdec.c starting at line 211), or at least gracefully fail without crashing.
> > Forbidden by whom? From what I can see:
> > - sample format: the wv demuxer does not set it at all, the decoder sets
> >   it in wavpack_decode_frame() from the block header; also, Michael sent
> >   a patch recently to deal with a wavpack decoder crash related to
> >   format changes
> > - sample rate: set by wv demuxer and then checked that it does not
> >   change; but the caller is not necessarily using the wv demuxer and the
> >   decoder will happily change it the same way it does with channel count
> >
> >> The reason it's never mentioned is probably just because it never occurred to me. None of the source file formats for WavPack
> >> (e.g., wav, caf, dsdiff) allow format changes and neither do any of the other lossless compressors I am aware of.
> > Correct, I cannot see any other lossless codecs that would allow this.
> > But then again all these codecs have global headers. So it's not that
> > they artificially forbid format changes, but the bistream format is not
> > able to represent them.
> 
> I disagree with that. It would be trivial to add a new STREAMINFO block to a FLAC file and change the format after that. This is
> not specifically prohibited in the FLAC specs, but it's sort of implied when it says that the initial STREAMINFO block has
> "information about the whole stream".

From a quick glance at the spec, metadata blocks are only allowed at the
beginning and are followed by audio frames. Once you see a single audio
frame, there should be no further metadata blocks.

That said, the frame header does contain stream parameters and those can
conceivably change and after looking at our decoder, it does support
those changes. It was added explicitly in
commit 90fcac0e95b7d266c148a86506f301a2072d9de3
Author: Justin Ruggles <justin.ruggles@gmail.com>
Date:   Sun Oct 21 17:02:28 2012 -0400

    flacdec: allow mid-stream channel layout change
    
    Although the libFLAC decoder cannot handle such a change, it is allowed by the
    spec and could potentially occur with live streams.

> 
> >
> >> and extensive fuzzing has ensured that pathological files like you
> >> suggest are unlikely to be the source of exploits.
> > See the crash above. I think fuzzing is very overhyped these days. It's
> > a useful tool for sure, but one should not rely purely on fuzzing too
> > much. It may not cover important parts of the input space because of the
> > way it is set up. Or there could be exploitable codepaths that require
> > too specific inputs for a random process to find, but a determined
> > attacker could construct them from reading the code.
> >
> Of course fuzzing cannot find everything, but my experience has been that fuzzing finds things that I would _never_ find by just
> looking at the code.
> 
> And I was certainly not suggesting that one should just write sloppy code knowing that all the bugs will be found with fuzzing.

Yeah, I certainly agree that fuzzing is a very useful tool. My point is
just that it shouldn't be your _only_ tool. There can be plenty of bugs
fuzzing won't find.
diff mbox series

Patch

diff --git a/doc/multithreading.txt b/doc/multithreading.txt
index 5b9dcb0bfc..4f645dc147 100644
--- a/doc/multithreading.txt
+++ b/doc/multithreading.txt
@@ -51,9 +51,6 @@  the decode process starts. Call ff_thread_finish_setup() afterwards. If
 some code can't be moved, have update_thread_context() run it in the next
 thread.
 
-If the codec allocates writable tables in its init(), add an init_thread_copy()
-which re-allocates them for other threads.
-
 Add AV_CODEC_CAP_FRAME_THREADS to the codec capabilities. There will be very little
 speed gain at this point but it should work.
 
diff --git a/libavcodec/aic.c b/libavcodec/aic.c
index 956d71fcff..f027fa99ef 100644
--- a/libavcodec/aic.c
+++ b/libavcodec/aic.c
@@ -504,6 +504,5 @@  AVCodec ff_aic_decoder = {
     .close          = aic_decode_close,
     .decode         = aic_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(aic_decode_init),
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/alac.c b/libavcodec/alac.c
index ea5ab182f9..c8c04223a0 100644
--- a/libavcodec/alac.c
+++ b/libavcodec/alac.c
@@ -601,15 +601,6 @@  static av_cold int alac_decode_init(AVCodecContext * avctx)
     return 0;
 }
 
-#if HAVE_THREADS
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    ALACContext *alac = avctx->priv_data;
-    alac->avctx = avctx;
-    return allocate_buffers(alac);
-}
-#endif
-
 static const AVOption options[] = {
     { "extra_bits_bug", "Force non-standard decoding process",
       offsetof(ALACContext, extra_bit_bug), AV_OPT_TYPE_BOOL, { .i64 = 0 },
@@ -633,7 +624,6 @@  AVCodec ff_alac_decoder = {
     .init           = alac_decode_init,
     .close          = alac_decode_close,
     .decode         = alac_decode_frame,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .priv_class     = &alac_class
 };
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 78c483c25c..6bf49c47e2 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3606,12 +3606,6 @@  typedef struct AVCodec {
      * @name Frame-level threading support functions
      * @{
      */
-    /**
-     * If defined, called on thread contexts when they are created.
-     * If the codec allocates writable tables in init(), re-allocate them here.
-     * priv_data will be set to a copy of the original.
-     */
-    int (*init_thread_copy)(AVCodecContext *);
     /**
      * Copy necessary context variables from a previous thread context to the current one.
      * If not defined, the next thread will start automatically; otherwise, the codec
diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index b4d6b25cbc..7956367b49 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -1039,10 +1039,8 @@  static av_cold int cfhd_close(AVCodecContext *avctx)
 
     free_buffers(s);
 
-    if (!avctx->internal->is_copy) {
-        ff_free_vlc(&s->vlc_9);
-        ff_free_vlc(&s->vlc_18);
-    }
+    ff_free_vlc(&s->vlc_9);
+    ff_free_vlc(&s->vlc_18);
 
     return 0;
 }
diff --git a/libavcodec/cllc.c b/libavcodec/cllc.c
index af0f6da2e9..1f2c98ef73 100644
--- a/libavcodec/cllc.c
+++ b/libavcodec/cllc.c
@@ -483,19 +483,6 @@  static int cllc_decode_frame(AVCodecContext *avctx, void *data,
     return avpkt->size;
 }
 
-#if HAVE_THREADS
-static int cllc_init_thread_copy(AVCodecContext *avctx)
-{
-    CLLCContext *ctx = avctx->priv_data;
-
-    ctx->avctx            = avctx;
-    ctx->swapped_buf      = NULL;
-    ctx->swapped_buf_size = 0;
-
-    return 0;
-}
-#endif
-
 static av_cold int cllc_decode_close(AVCodecContext *avctx)
 {
     CLLCContext *ctx = avctx->priv_data;
@@ -526,7 +513,6 @@  AVCodec ff_cllc_decoder = {
     .id             = AV_CODEC_ID_CLLC,
     .priv_data_size = sizeof(CLLCContext),
     .init           = cllc_decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(cllc_init_thread_copy),
     .decode         = cllc_decode_frame,
     .close          = cllc_decode_close,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
diff --git a/libavcodec/dnxhddec.c b/libavcodec/dnxhddec.c
index f7585458b9..e5d01e2e71 100644
--- a/libavcodec/dnxhddec.c
+++ b/libavcodec/dnxhddec.c
@@ -144,21 +144,6 @@  static int dnxhd_init_vlc(DNXHDContext *ctx, uint32_t cid, int bitdepth)
     return 0;
 }
 
-static av_cold int dnxhd_decode_init_thread_copy(AVCodecContext *avctx)
-{
-    DNXHDContext *ctx = avctx->priv_data;
-
-    ctx->avctx = avctx;
-    // make sure VLC tables will be loaded when cid is parsed
-    ctx->cid = -1;
-
-    ctx->rows = av_mallocz_array(avctx->thread_count, sizeof(RowContext));
-    if (!ctx->rows)
-        return AVERROR(ENOMEM);
-
-    return 0;
-}
-
 static int dnxhd_get_profile(int cid)
 {
     switch(cid) {
@@ -740,6 +725,5 @@  AVCodec ff_dnxhd_decoder = {
     .decode         = dnxhd_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS |
                       AV_CODEC_CAP_SLICE_THREADS,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(dnxhd_decode_init_thread_copy),
     .profiles       = NULL_IF_CONFIG_SMALL(ff_dnxhd_profiles),
 };
diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 1db30a1ae0..73419eadb1 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1863,20 +1863,6 @@  static av_cold int decode_init(AVCodecContext *avctx)
     return 0;
 }
 
-#if HAVE_THREADS
-static int decode_init_thread_copy(AVCodecContext *avctx)
-{
-    EXRContext *s = avctx->priv_data;
-
-    // allocate thread data, used for non EXR_RAW compression types
-    s->thread_data = av_mallocz_array(avctx->thread_count, sizeof(EXRThreadData));
-    if (!s->thread_data)
-        return AVERROR_INVALIDDATA;
-
-    return 0;
-}
-#endif
-
 static av_cold int decode_end(AVCodecContext *avctx)
 {
     EXRContext *s = avctx->priv_data;
@@ -1956,7 +1942,6 @@  AVCodec ff_exr_decoder = {
     .id               = AV_CODEC_ID_EXR,
     .priv_data_size   = sizeof(EXRContext),
     .init             = decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
     .close            = decode_end,
     .decode           = decode_frame,
     .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS |
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 2ffd3ef991..c704373cfe 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -977,34 +977,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
     return buf_size;
 }
 
-#if HAVE_THREADS
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    FFV1Context *f = avctx->priv_data;
-    int i, ret;
-
-    f->picture.f      = NULL;
-    f->last_picture.f = NULL;
-    f->sample_buffer  = NULL;
-    f->max_slice_count = 0;
-    f->slice_count = 0;
-
-    for (i = 0; i < f->quant_table_count; i++) {
-        av_assert0(f->version > 1);
-        f->initial_states[i] = av_memdup(f->initial_states[i],
-                                         f->context_count[i] * sizeof(*f->initial_states[i]));
-    }
-
-    f->picture.f      = av_frame_alloc();
-    f->last_picture.f = av_frame_alloc();
-
-    if ((ret = ff_ffv1_init_slice_contexts(f)) < 0)
-        return ret;
-
-    return 0;
-}
-#endif
-
 static void copy_fields(FFV1Context *fsdst, FFV1Context *fssrc, FFV1Context *fsrc)
 {
     fsdst->version             = fsrc->version;
@@ -1088,7 +1060,6 @@  AVCodec ff_ffv1_decoder = {
     .init           = decode_init,
     .close          = ff_ffv1_close,
     .decode         = decode_frame,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
     .capabilities   = AV_CODEC_CAP_DR1 /*| AV_CODEC_CAP_DRAW_HORIZ_BAND*/ |
                       AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS,
diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
index c8eb456049..fb27e8e6d4 100644
--- a/libavcodec/flacdec.c
+++ b/libavcodec/flacdec.c
@@ -639,19 +639,6 @@  static int flac_decode_frame(AVCodecContext *avctx, void *data,
     return bytes_read;
 }
 
-#if HAVE_THREADS
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    FLACContext *s = avctx->priv_data;
-    s->decoded_buffer = NULL;
-    s->decoded_buffer_size = 0;
-    s->avctx = avctx;
-    if (s->flac_stream_info.max_blocksize)
-        return allocate_buffers(s);
-    return 0;
-}
-#endif
-
 static av_cold int flac_decode_close(AVCodecContext *avctx)
 {
     FLACContext *s = avctx->priv_data;
@@ -682,7 +669,6 @@  AVCodec ff_flac_decoder = {
     .init           = flac_decode_init,
     .close          = flac_decode_close,
     .decode         = flac_decode_frame,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .sample_fmts    = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16,
                                                       AV_SAMPLE_FMT_S16P,
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index defe514828..b6c51ed1e2 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -409,13 +409,15 @@  static av_cold int h264_decode_init(AVCodecContext *avctx)
     }
     avctx->ticks_per_frame = 2;
 
-    if (avctx->extradata_size > 0 && avctx->extradata) {
-        ret = ff_h264_decode_extradata(avctx->extradata, avctx->extradata_size,
-                                       &h->ps, &h->is_avc, &h->nal_length_size,
-                                       avctx->err_recognition, avctx);
-        if (ret < 0) {
-            h264_decode_end(avctx);
-            return ret;
+    if (!avctx->internal->is_copy) {
+        if (avctx->extradata_size > 0 && avctx->extradata) {
+            ret = ff_h264_decode_extradata(avctx->extradata, avctx->extradata_size,
+                                           &h->ps, &h->is_avc, &h->nal_length_size,
+                                           avctx->err_recognition, avctx);
+            if (ret < 0) {
+                h264_decode_end(avctx);
+                return ret;
+            }
         }
     }
 
@@ -438,27 +440,6 @@  static av_cold int h264_decode_init(AVCodecContext *avctx)
     return 0;
 }
 
-#if HAVE_THREADS
-static int decode_init_thread_copy(AVCodecContext *avctx)
-{
-    H264Context *h = avctx->priv_data;
-    int ret;
-
-    if (!avctx->internal->is_copy)
-        return 0;
-
-    memset(h, 0, sizeof(*h));
-
-    ret = h264_init_context(avctx, h);
-    if (ret < 0)
-        return ret;
-
-    h->context_initialized = 0;
-
-    return 0;
-}
-#endif
-
 /**
  * instantaneous decoder refresh.
  */
@@ -1081,7 +1062,6 @@  AVCodec ff_h264_decoder = {
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_EXPORTS_CROPPING |
                              FF_CODEC_CAP_ALLOCATE_PROGRESS,
     .flush                 = flush_dpb,
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(ff_h264_update_thread_context),
     .profiles              = NULL_IF_CONFIG_SMALL(ff_h264_profiles),
     .priv_class            = &h264_class,
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 58689a4370..36be83948e 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3506,11 +3506,13 @@  static av_cold int hevc_decode_init(AVCodecContext *avctx)
     else
         s->threads_number = 1;
 
-    if (avctx->extradata_size > 0 && avctx->extradata) {
-        ret = hevc_decode_extradata(s, avctx->extradata, avctx->extradata_size, 1);
-        if (ret < 0) {
-            hevc_decode_free(avctx);
-            return ret;
+    if (!avctx->internal->is_copy) {
+        if (avctx->extradata_size > 0 && avctx->extradata) {
+            ret = hevc_decode_extradata(s, avctx->extradata, avctx->extradata_size, 1);
+            if (ret < 0) {
+                hevc_decode_free(avctx);
+                return ret;
+            }
         }
     }
 
@@ -3522,22 +3524,6 @@  static av_cold int hevc_decode_init(AVCodecContext *avctx)
     return 0;
 }
 
-#if HAVE_THREADS
-static av_cold int hevc_init_thread_copy(AVCodecContext *avctx)
-{
-    HEVCContext *s = avctx->priv_data;
-    int ret;
-
-    memset(s, 0, sizeof(*s));
-
-    ret = hevc_init_context(avctx);
-    if (ret < 0)
-        return ret;
-
-    return 0;
-}
-#endif
-
 static void hevc_decode_flush(AVCodecContext *avctx)
 {
     HEVCContext *s = avctx->priv_data;
@@ -3577,7 +3563,6 @@  AVCodec ff_hevc_decoder = {
     .decode                = hevc_decode_frame,
     .flush                 = hevc_decode_flush,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(hevc_update_thread_context),
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(hevc_init_thread_copy),
     .capabilities          = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
                              AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS,
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_EXPORTS_CROPPING |
diff --git a/libavcodec/hqx.c b/libavcodec/hqx.c
index 39404d24e5..e2b895ac40 100644
--- a/libavcodec/hqx.c
+++ b/libavcodec/hqx.c
@@ -520,9 +520,6 @@  static av_cold int hqx_decode_close(AVCodecContext *avctx)
     int i;
     HQXContext *ctx = avctx->priv_data;
 
-    if (avctx->internal->is_copy)
-        return 0;
-
     ff_free_vlc(&ctx->cbp_vlc);
     for (i = 0; i < 3; i++) {
         ff_free_vlc(&ctx->dc_vlc[i]);
diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c
index 46dcfa8235..0ee7ec3917 100644
--- a/libavcodec/huffyuvdec.c
+++ b/libavcodec/huffyuvdec.c
@@ -570,35 +570,6 @@  static av_cold int decode_init(AVCodecContext *avctx)
     return ret;
 }
 
-#if HAVE_THREADS
-static av_cold int decode_init_thread_copy(AVCodecContext *avctx)
-{
-    HYuvContext *s = avctx->priv_data;
-    int i, ret;
-
-    s->avctx = avctx;
-
-    if ((ret = ff_huffyuv_alloc_temp(s)) < 0) {
-        ff_huffyuv_common_end(s);
-        return ret;
-    }
-
-    for (i = 0; i < 8; i++)
-        s->vlc[i].table = NULL;
-
-    if (s->version >= 2) {
-        if ((ret = read_huffman_tables(s, avctx->extradata + 4,
-                                       avctx->extradata_size)) < 0)
-            return ret;
-    } else {
-        if ((ret = read_old_huffman_tables(s)) < 0)
-            return ret;
-    }
-
-    return 0;
-}
-#endif
-
 /** Subset of GET_VLC for use in hand-roller VLC code */
 #define VLC_INTERN(dst, table, gb, name, bits, max_depth)   \
     code = table[index][0];                                 \
@@ -1302,7 +1273,6 @@  AVCodec ff_huffyuv_decoder = {
     .decode           = decode_frame,
     .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DRAW_HORIZ_BAND |
                         AV_CODEC_CAP_FRAME_THREADS,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
 };
 
 #if CONFIG_FFVHUFF_DECODER
@@ -1317,7 +1287,6 @@  AVCodec ff_ffvhuff_decoder = {
     .decode           = decode_frame,
     .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DRAW_HORIZ_BAND |
                         AV_CODEC_CAP_FRAME_THREADS,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
 };
 #endif /* CONFIG_FFVHUFF_DECODER */
 
@@ -1333,6 +1302,5 @@  AVCodec ff_hymt_decoder = {
     .decode           = decode_frame,
     .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DRAW_HORIZ_BAND |
                         AV_CODEC_CAP_FRAME_THREADS,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
 };
 #endif /* CONFIG_HYMT_DECODER */
diff --git a/libavcodec/lagarith.c b/libavcodec/lagarith.c
index 0a45812bc1..d81e55cf4c 100644
--- a/libavcodec/lagarith.c
+++ b/libavcodec/lagarith.c
@@ -712,16 +712,6 @@  static av_cold int lag_decode_init(AVCodecContext *avctx)
     return 0;
 }
 
-#if HAVE_THREADS
-static av_cold int lag_decode_init_thread_copy(AVCodecContext *avctx)
-{
-    LagarithContext *l = avctx->priv_data;
-    l->avctx = avctx;
-
-    return 0;
-}
-#endif
-
 AVCodec ff_lagarith_decoder = {
     .name           = "lagarith",
     .long_name      = NULL_IF_CONFIG_SMALL("Lagarith lossless"),
@@ -729,7 +719,6 @@  AVCodec ff_lagarith_decoder = {
     .id             = AV_CODEC_ID_LAGARITH,
     .priv_data_size = sizeof(LagarithContext),
     .init           = lag_decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(lag_decode_init_thread_copy),
     .decode         = lag_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
 };
diff --git a/libavcodec/lcldec.c b/libavcodec/lcldec.c
index 046cdc4f8e..c51083bdf2 100644
--- a/libavcodec/lcldec.c
+++ b/libavcodec/lcldec.c
@@ -622,13 +622,6 @@  static av_cold int decode_init(AVCodecContext *avctx)
     return 0;
 }
 
-#if HAVE_THREADS
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    return decode_init(avctx);
-}
-#endif
-
 static av_cold int decode_end(AVCodecContext *avctx)
 {
     LclDecContext * const c = avctx->priv_data;
@@ -650,7 +643,6 @@  AVCodec ff_mszh_decoder = {
     .id             = AV_CODEC_ID_MSZH,
     .priv_data_size = sizeof(LclDecContext),
     .init           = decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .close          = decode_end,
     .decode         = decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
@@ -666,7 +658,6 @@  AVCodec ff_zlib_decoder = {
     .id             = AV_CODEC_ID_ZLIB,
     .priv_data_size = sizeof(LclDecContext),
     .init           = decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .close          = decode_end,
     .decode         = decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index aacd0d4d7d..5d76274d54 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -749,21 +749,6 @@  static int magy_decode_frame(AVCodecContext *avctx, void *data,
     return avpkt->size;
 }
 
-#if HAVE_THREADS
-static int magy_init_thread_copy(AVCodecContext *avctx)
-{
-    MagicYUVContext *s = avctx->priv_data;
-    int i;
-
-    for (i = 0; i < FF_ARRAY_ELEMS(s->slices); i++) {
-        s->slices[i] = NULL;
-        s->slices_size[i] = 0;
-    }
-
-    return 0;
-}
-#endif
-
 static av_cold int magy_decode_init(AVCodecContext *avctx)
 {
     MagicYUVContext *s = avctx->priv_data;
@@ -792,7 +777,6 @@  AVCodec ff_magicyuv_decoder = {
     .id               = AV_CODEC_ID_MAGICYUV,
     .priv_data_size   = sizeof(MagicYUVContext),
     .init             = magy_decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(magy_init_thread_copy),
     .close            = magy_decode_end,
     .decode           = magy_decode_frame,
     .capabilities     = AV_CODEC_CAP_DR1 |
diff --git a/libavcodec/mdec.c b/libavcodec/mdec.c
index 330b761279..7e34ec568e 100644
--- a/libavcodec/mdec.c
+++ b/libavcodec/mdec.c
@@ -240,17 +240,6 @@  static av_cold int decode_init(AVCodecContext *avctx)
     return 0;
 }
 
-#if HAVE_THREADS
-static av_cold int decode_init_thread_copy(AVCodecContext *avctx)
-{
-    MDECContext * const a = avctx->priv_data;
-
-    a->avctx           = avctx;
-
-    return 0;
-}
-#endif
-
 static av_cold int decode_end(AVCodecContext *avctx)
 {
     MDECContext * const a = avctx->priv_data;
@@ -271,5 +260,4 @@  AVCodec ff_mdec_decoder = {
     .close            = decode_end,
     .decode           = decode_frame,
     .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy)
 };
diff --git a/libavcodec/mimic.c b/libavcodec/mimic.c
index 8e3a77a334..2563a49d53 100644
--- a/libavcodec/mimic.c
+++ b/libavcodec/mimic.c
@@ -128,8 +128,7 @@  static av_cold int mimic_decode_end(AVCodecContext *avctx)
         av_frame_free(&ctx->frames[i].f);
     }
 
-    if (!avctx->internal->is_copy)
-        ff_free_vlc(&ctx->vlc);
+    ff_free_vlc(&ctx->vlc);
 
     return 0;
 }
@@ -449,24 +448,6 @@  static int mimic_decode_frame(AVCodecContext *avctx, void *data,
     return buf_size;
 }
 
-#if HAVE_THREADS
-static av_cold int mimic_init_thread_copy(AVCodecContext *avctx)
-{
-    MimicContext *ctx = avctx->priv_data;
-    int i;
-
-    for (i = 0; i < FF_ARRAY_ELEMS(ctx->frames); i++) {
-        ctx->frames[i].f = av_frame_alloc();
-        if (!ctx->frames[i].f) {
-            mimic_decode_end(avctx);
-            return AVERROR(ENOMEM);
-        }
-    }
-
-    return 0;
-}
-#endif
-
 AVCodec ff_mimic_decoder = {
     .name                  = "mimic",
     .long_name             = NULL_IF_CONFIG_SMALL("Mimic"),
@@ -478,6 +459,5 @@  AVCodec ff_mimic_decoder = {
     .decode                = mimic_decode_frame,
     .capabilities          = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(mimic_decode_update_thread_context),
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(mimic_init_thread_copy),
     .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index de35b330ad..bfb1f92b33 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -3559,13 +3559,11 @@  static av_cold int decode_end(AVCodecContext *avctx)
     Mpeg4DecContext *ctx = avctx->priv_data;
     int i;
 
-    if (!avctx->internal->is_copy) {
-        for (i = 0; i < 12; i++)
-            ff_free_vlc(&ctx->studio_intra_tab[i]);
+    for (i = 0; i < 12; i++)
+        ff_free_vlc(&ctx->studio_intra_tab[i]);
 
-        ff_free_vlc(&ctx->studio_luma_dc);
-        ff_free_vlc(&ctx->studio_chroma_dc);
-    }
+    ff_free_vlc(&ctx->studio_luma_dc);
+    ff_free_vlc(&ctx->studio_chroma_dc);
 
     return ff_h263_decode_end(avctx);
 }
diff --git a/libavcodec/pixlet.c b/libavcodec/pixlet.c
index 03a2cdacc8..7b068b1ce5 100644
--- a/libavcodec/pixlet.c
+++ b/libavcodec/pixlet.c
@@ -675,28 +675,12 @@  static int pixlet_decode_frame(AVCodecContext *avctx, void *data,
     return pktsize;
 }
 
-#if HAVE_THREADS
-static int pixlet_init_thread_copy(AVCodecContext *avctx)
-{
-    PixletContext *ctx = avctx->priv_data;
-
-    ctx->filter[0]  = NULL;
-    ctx->filter[1]  = NULL;
-    ctx->prediction = NULL;
-    ctx->w = 0;
-    ctx->h = 0;
-
-    return 0;
-}
-#endif /* HAVE_THREADS */
-
 AVCodec ff_pixlet_decoder = {
     .name             = "pixlet",
     .long_name        = NULL_IF_CONFIG_SMALL("Apple Pixlet"),
     .type             = AVMEDIA_TYPE_VIDEO,
     .id               = AV_CODEC_ID_PIXLET,
     .init             = pixlet_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(pixlet_init_thread_copy),
     .close            = pixlet_close,
     .decode           = pixlet_decode_frame,
     .priv_data_size   = sizeof(PixletContext),
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index af8608b23b..7e2c19bd57 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -1771,9 +1771,7 @@  static av_cold int png_dec_init(AVCodecContext *avctx)
         return AVERROR(ENOMEM);
     }
 
-    if (!avctx->internal->is_copy) {
-        ff_pngdsp_init(&s->dsp);
-    }
+    ff_pngdsp_init(&s->dsp);
 
     return 0;
 }
@@ -1808,7 +1806,6 @@  AVCodec ff_apng_decoder = {
     .init           = png_dec_init,
     .close          = png_dec_end,
     .decode         = decode_frame_apng,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(png_dec_init),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS /*| AV_CODEC_CAP_DRAW_HORIZ_BAND*/,
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
@@ -1826,7 +1823,6 @@  AVCodec ff_png_decoder = {
     .init           = png_dec_init,
     .close          = png_dec_end,
     .decode         = decode_frame_png,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(png_dec_init),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS /*| AV_CODEC_CAP_DRAW_HORIZ_BAND*/,
     .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_THREADSAFE |
diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
index 2652a31c81..d5fbfc6711 100644
--- a/libavcodec/proresdec2.c
+++ b/libavcodec/proresdec2.c
@@ -807,17 +807,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     return avpkt->size;
 }
 
-#if HAVE_THREADS
-static int decode_init_thread_copy(AVCodecContext *avctx)
-{
-    ProresContext *ctx = avctx->priv_data;
-
-    ctx->slices = NULL;
-
-    return 0;
-}
-#endif
-
 static av_cold int decode_close(AVCodecContext *avctx)
 {
     ProresContext *ctx = avctx->priv_data;
@@ -834,7 +823,6 @@  AVCodec ff_prores_decoder = {
     .id             = AV_CODEC_ID_PRORES,
     .priv_data_size = sizeof(ProresContext),
     .init           = decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
     .close          = decode_close,
     .decode         = decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS,
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index ccccabf4a6..183d0f84f2 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -703,7 +703,10 @@  void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
         av_freep(&p->released_buffers);
 
         if (i && p->avctx) {
+            if (codec->priv_class)
+                av_opt_free(p->avctx->priv_data);
             av_freep(&p->avctx->priv_data);
+
             av_freep(&p->avctx->slice_offset);
         }
 
@@ -809,28 +812,30 @@  int ff_frame_thread_init(AVCodecContext *avctx)
         copy->internal->thread_ctx = p;
         copy->internal->last_pkt_props = &p->avpkt;
 
-        if (!i) {
-            src = copy;
-
-            if (codec->init)
-                err = codec->init(copy);
-
-            update_context_from_thread(avctx, copy, 1);
-        } else {
-            copy->priv_data = av_malloc(codec->priv_data_size);
+        if (i) {
+            copy->priv_data = av_mallocz(codec->priv_data_size);
             if (!copy->priv_data) {
                 err = AVERROR(ENOMEM);
                 goto error;
             }
-            memcpy(copy->priv_data, src->priv_data, codec->priv_data_size);
-            copy->internal->is_copy = 1;
 
-            if (codec->init_thread_copy)
-                err = codec->init_thread_copy(copy);
+            if (codec->priv_class) {
+                *(const AVClass **)copy->priv_data = codec->priv_class;
+                err = av_opt_copy(copy->priv_data, src->priv_data);
+                if (err < 0)
+                    goto error;
+            }
+            copy->internal->is_copy = 1;
         }
 
+        if (codec->init)
+            err = codec->init(copy);
+
         if (err) goto error;
 
+        if (!i)
+            update_context_from_thread(avctx, copy, 1);
+
         atomic_init(&p->debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0);
 
         err = AVERROR(pthread_create(&p->thread, NULL, frame_worker_thread, p));
diff --git a/libavcodec/rv30.c b/libavcodec/rv30.c
index 0a63352e6b..36cd5345fd 100644
--- a/libavcodec/rv30.c
+++ b/libavcodec/rv30.c
@@ -304,7 +304,6 @@  AVCodec ff_rv30_decoder = {
         AV_PIX_FMT_YUV420P,
         AV_PIX_FMT_NONE
     },
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(ff_rv34_decode_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(ff_rv34_decode_update_thread_context),
     .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c
index f877258db6..ec0cd27916 100644
--- a/libavcodec/rv34.c
+++ b/libavcodec/rv34.c
@@ -1529,34 +1529,6 @@  av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
     return 0;
 }
 
-int ff_rv34_decode_init_thread_copy(AVCodecContext *avctx)
-{
-    int err;
-    RV34DecContext *r = avctx->priv_data;
-
-    r->s.avctx = avctx;
-
-    if (avctx->internal->is_copy) {
-        r->tmp_b_block_base = NULL;
-        r->cbp_chroma       = NULL;
-        r->cbp_luma         = NULL;
-        r->deblock_coefs    = NULL;
-        r->intra_types_hist = NULL;
-        r->mb_type          = NULL;
-
-        ff_mpv_idct_init(&r->s);
-
-        if ((err = ff_mpv_common_init(&r->s)) < 0)
-            return err;
-        if ((err = rv34_decoder_alloc(r)) < 0) {
-            ff_mpv_common_end(&r->s);
-            return err;
-        }
-    }
-
-    return 0;
-}
-
 int ff_rv34_decode_update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
 {
     RV34DecContext *r = dst->priv_data, *r1 = src->priv_data;
diff --git a/libavcodec/rv34.h b/libavcodec/rv34.h
index efff94a1d9..1d5522538b 100644
--- a/libavcodec/rv34.h
+++ b/libavcodec/rv34.h
@@ -136,7 +136,6 @@  int ff_rv34_get_start_offset(GetBitContext *gb, int blocks);
 int ff_rv34_decode_init(AVCodecContext *avctx);
 int ff_rv34_decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPacket *avpkt);
 int ff_rv34_decode_end(AVCodecContext *avctx);
-int ff_rv34_decode_init_thread_copy(AVCodecContext *avctx);
 int ff_rv34_decode_update_thread_context(AVCodecContext *dst, const AVCodecContext *src);
 
 #endif /* AVCODEC_RV34_H */
diff --git a/libavcodec/rv40.c b/libavcodec/rv40.c
index ad15044ee3..462024c81e 100644
--- a/libavcodec/rv40.c
+++ b/libavcodec/rv40.c
@@ -583,7 +583,6 @@  AVCodec ff_rv40_decoder = {
         AV_PIX_FMT_YUV420P,
         AV_PIX_FMT_NONE
     },
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(ff_rv34_decode_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(ff_rv34_decode_update_thread_context),
     .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
diff --git a/libavcodec/sheervideo.c b/libavcodec/sheervideo.c
index 50c3ebcee7..1a43727a30 100644
--- a/libavcodec/sheervideo.c
+++ b/libavcodec/sheervideo.c
@@ -2063,19 +2063,6 @@  static int decode_frame(AVCodecContext *avctx,
     return avpkt->size;
 }
 
-#if HAVE_THREADS
-static int decode_init_thread_copy(AVCodecContext *avctx)
-{
-    SheerVideoContext *s = avctx->priv_data;
-
-    s->format = 0;
-    memset(&s->vlc[0], 0, sizeof(s->vlc[0]));
-    memset(&s->vlc[1], 0, sizeof(s->vlc[1]));
-
-    return 0;
-}
-#endif
-
 static av_cold int decode_end(AVCodecContext *avctx)
 {
     SheerVideoContext *s = avctx->priv_data;
@@ -2092,7 +2079,6 @@  AVCodec ff_sheervideo_decoder = {
     .type             = AVMEDIA_TYPE_VIDEO,
     .id               = AV_CODEC_ID_SHEERVIDEO,
     .priv_data_size   = sizeof(SheerVideoContext),
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
     .close            = decode_end,
     .decode           = decode_frame,
     .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
diff --git a/libavcodec/takdec.c b/libavcodec/takdec.c
index 8ec87ab509..9fa1cb1f7f 100644
--- a/libavcodec/takdec.c
+++ b/libavcodec/takdec.c
@@ -915,13 +915,6 @@  static int tak_decode_frame(AVCodecContext *avctx, void *data,
 }
 
 #if HAVE_THREADS
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    TAKDecContext *s = avctx->priv_data;
-    s->avctx = avctx;
-    return 0;
-}
-
 static int update_thread_context(AVCodecContext *dst,
                                  const AVCodecContext *src)
 {
@@ -953,7 +946,6 @@  AVCodec ff_tak_decoder = {
     .init             = tak_decode_init,
     .close            = tak_decode_close,
     .decode           = tak_decode_frame,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
     .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .sample_fmts      = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_U8P,
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index e8357114de..3b5985ed1e 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -2165,7 +2165,6 @@  AVCodec ff_tiff_decoder = {
     .init           = tiff_init,
     .close          = tiff_end,
     .decode         = decode_frame,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(tiff_init),
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
     .priv_class     = &tiff_decoder_class,
diff --git a/libavcodec/tta.c b/libavcodec/tta.c
index 3fbee06987..e68e4fbb36 100644
--- a/libavcodec/tta.c
+++ b/libavcodec/tta.c
@@ -389,13 +389,6 @@  error:
     return ret;
 }
 
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    TTAContext *s = avctx->priv_data;
-    s->avctx = avctx;
-    return allocate_buffers(avctx);
-}
-
 static av_cold int tta_decode_close(AVCodecContext *avctx) {
     TTAContext *s = avctx->priv_data;
 
@@ -430,7 +423,6 @@  AVCodec ff_tta_decoder = {
     .init           = tta_decode_init,
     .close          = tta_decode_close,
     .decode         = tta_decode_frame,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .priv_class     = &tta_decoder_class,
 };
diff --git a/libavcodec/vble.c b/libavcodec/vble.c
index c25ee98697..c48c13127a 100644
--- a/libavcodec/vble.c
+++ b/libavcodec/vble.c
@@ -214,6 +214,5 @@  AVCodec ff_vble_decoder = {
     .close          = vble_decode_close,
     .decode         = vble_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(vble_decode_init),
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
index 8e37ba5fa0..81d0b9b7bb 100644
--- a/libavcodec/vp3.c
+++ b/libavcodec/vp3.c
@@ -347,9 +347,6 @@  static av_cold int vp3_decode_end(AVCodecContext *avctx)
     av_frame_free(&s->last_frame.f);
     av_frame_free(&s->golden_frame.f);
 
-    if (avctx->internal->is_copy)
-        return 0;
-
     for (i = 0; i < 16; i++) {
         ff_free_vlc(&s->dc_vlc[i]);
         ff_free_vlc(&s->ac_vlc_1[i]);
@@ -2601,23 +2598,6 @@  static int vp3_update_thread_context(AVCodecContext *dst, const AVCodecContext *
     }
 
     if (s != s1) {
-        if (!s->current_frame.f)
-            return AVERROR(ENOMEM);
-        // init tables if the first frame hasn't been decoded
-        if (!s->current_frame.f->data[0]) {
-            int y_fragment_count, c_fragment_count;
-            s->avctx = dst;
-            err = allocate_tables(dst);
-            if (err)
-                return err;
-            y_fragment_count = s->fragment_width[0] * s->fragment_height[0];
-            c_fragment_count = s->fragment_width[1] * s->fragment_height[1];
-            memcpy(s->motion_val[0], s1->motion_val[0],
-                   y_fragment_count * sizeof(*s->motion_val[0]));
-            memcpy(s->motion_val[1], s1->motion_val[1],
-                   c_fragment_count * sizeof(*s->motion_val[1]));
-        }
-
         // copy previous frame data
         if ((err = ref_frames(s, s1)) < 0)
             return err;
@@ -2927,28 +2907,6 @@  static int read_huffman_tree(AVCodecContext *avctx, GetBitContext *gb)
     return 0;
 }
 
-#if HAVE_THREADS
-static int vp3_init_thread_copy(AVCodecContext *avctx)
-{
-    Vp3DecodeContext *s = avctx->priv_data;
-
-    s->superblock_coding      = NULL;
-    s->all_fragments          = NULL;
-    s->coded_fragment_list[0] = NULL;
-    s-> kf_coded_fragment_list= NULL;
-    s->nkf_coded_fragment_list= NULL;
-    s->dct_tokens_base        = NULL;
-    s->superblock_fragments   = NULL;
-    s->macroblock_coding      = NULL;
-    s->motion_val[0]          = NULL;
-    s->motion_val[1]          = NULL;
-    s->edge_emu_buffer        = NULL;
-    s->dc_pred_row            = NULL;
-
-    return init_frames(s);
-}
-#endif
-
 #if CONFIG_THEORA_DECODER
 static const enum AVPixelFormat theora_pix_fmts[4] = {
     AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV444P
@@ -3262,7 +3220,6 @@  AVCodec ff_theora_decoder = {
     .capabilities          = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DRAW_HORIZ_BAND |
                              AV_CODEC_CAP_FRAME_THREADS,
     .flush                 = vp3_decode_flush,
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(vp3_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
     .caps_internal         = FF_CODEC_CAP_EXPORTS_CROPPING | FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
@@ -3280,7 +3237,6 @@  AVCodec ff_vp3_decoder = {
     .capabilities          = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DRAW_HORIZ_BAND |
                              AV_CODEC_CAP_FRAME_THREADS,
     .flush                 = vp3_decode_flush,
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(vp3_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
     .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
@@ -3298,7 +3254,6 @@  AVCodec ff_vp4_decoder = {
     .capabilities          = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DRAW_HORIZ_BAND |
                              AV_CODEC_CAP_FRAME_THREADS,
     .flush                 = vp3_decode_flush,
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(vp3_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
     .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 81da0422be..1794d6d031 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -2894,21 +2894,6 @@  av_cold int ff_vp8_decode_init(AVCodecContext *avctx)
 
 #if CONFIG_VP8_DECODER
 #if HAVE_THREADS
-static av_cold int vp8_decode_init_thread_copy(AVCodecContext *avctx)
-{
-    VP8Context *s = avctx->priv_data;
-    int ret;
-
-    s->avctx = avctx;
-
-    if ((ret = vp8_init_frames(s)) < 0) {
-        ff_vp8_decode_free(avctx);
-        return ret;
-    }
-
-    return 0;
-}
-
 #define REBASE(pic) ((pic) ? (pic) - &s_src->frames[0] + &s->frames[0] : NULL)
 
 static int vp8_decode_update_thread_context(AVCodecContext *dst,
@@ -2976,7 +2961,6 @@  AVCodec ff_vp8_decoder = {
     .capabilities          = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS |
                              AV_CODEC_CAP_SLICE_THREADS,
     .flush                 = vp8_decode_flush,
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(vp8_decode_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp8_decode_update_thread_context),
     .hw_configs            = (const AVCodecHWConfigInternal*[]) {
 #if CONFIG_VP8_VAAPI_HWACCEL
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index e47ed4e04e..2a3a4555b9 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -1748,11 +1748,6 @@  static av_cold int vp9_decode_init(AVCodecContext *avctx)
 }
 
 #if HAVE_THREADS
-static av_cold int vp9_decode_init_thread_copy(AVCodecContext *avctx)
-{
-    return init_frames(avctx);
-}
-
 static int vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
 {
     int i, ret;
@@ -1812,7 +1807,6 @@  AVCodec ff_vp9_decoder = {
     .caps_internal         = FF_CODEC_CAP_SLICE_THREAD_HAS_MF |
                              FF_CODEC_CAP_ALLOCATE_PROGRESS,
     .flush                 = vp9_decode_flush,
-    .init_thread_copy      = ONLY_IF_THREADS_ENABLED(vp9_decode_init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp9_decode_update_thread_context),
     .profiles              = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
     .bsfs                  = "vp9_superframe_split",
diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 2a2a2b7690..d96ee1fa3a 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -979,20 +979,6 @@  static av_cold int wv_alloc_frame_context(WavpackContext *c)
 }
 
 #if HAVE_THREADS
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    WavpackContext *s = avctx->priv_data;
-    s->avctx = avctx;
-
-    s->curr_frame.f = av_frame_alloc();
-    s->prev_frame.f = av_frame_alloc();
-
-    if (!s->curr_frame.f || !s->prev_frame.f)
-        return AVERROR(ENOMEM);
-
-    return 0;
-}
-
 static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
 {
     WavpackContext *fsrc = src->priv_data;
@@ -1656,7 +1642,6 @@  AVCodec ff_wavpack_decoder = {
     .close          = wavpack_decode_end,
     .decode         = wavpack_decode_frame,
     .flush          = wavpack_decode_flush,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS |
                       AV_CODEC_CAP_SLICE_THREADS,
diff --git a/libavcodec/ylc.c b/libavcodec/ylc.c
index 11333222b9..2afe3fc9d5 100644
--- a/libavcodec/ylc.c
+++ b/libavcodec/ylc.c
@@ -453,24 +453,6 @@  static int decode_frame(AVCodecContext *avctx,
     return avpkt->size;
 }
 
-#if HAVE_THREADS
-static int init_thread_copy(AVCodecContext *avctx)
-{
-    YLCContext *s = avctx->priv_data;
-
-    memset(&s->vlc[0], 0, sizeof(VLC));
-    memset(&s->vlc[1], 0, sizeof(VLC));
-    memset(&s->vlc[2], 0, sizeof(VLC));
-    memset(&s->vlc[3], 0, sizeof(VLC));
-    s->table_bits = NULL;
-    s->table_bits_size = 0;
-    s->bitstream_bits = NULL;
-    s->bitstream_bits_size = 0;
-
-    return 0;
-}
-#endif
-
 static av_cold int decode_end(AVCodecContext *avctx)
 {
     YLCContext *s = avctx->priv_data;
@@ -494,7 +476,6 @@  AVCodec ff_ylc_decoder = {
     .id             = AV_CODEC_ID_YLC,
     .priv_data_size = sizeof(YLCContext),
     .init           = decode_init,
-    .init_thread_copy = ONLY_IF_THREADS_ENABLED(init_thread_copy),
     .close          = decode_end,
     .decode         = decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,