Recherche avancée

Sur d’autres sites (12426)

• avcodec/mpeg4videodec : Fix data race when initializing VLCs

  4 janvier 2022, par Andreas Rheinhardt

  avcodec/mpeg4videodec : Fix data race when initializing VLCs
  Both the MPEG-4 parser as well as the decoder initialized
  
  several VLCs. There is a "static int done = 0 ;" in order to
  
  guard against initializing these multiple times, but this does
  
  not work when several threads try to initialize these VLCs
  
  concurrently, which can happen when initializing several parsers
  
  at the same time (they don't use the global lock that is used
  
  for codecs without the FF_CODEC_CAP_INIT_THREADSAFE cap ; actually,
  
  they don't use any lock at all).
  Since ff_mpeg4_decode_picture_header() now aborts early when called
  
  from the parser, it no longer needs to have these VLCs initialized
  
  at all. This commit therefore does exactly this.
  Signed-off-by : Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
  

  • [DH] libavcodec/mpeg4video.h
  • [DH] libavcodec/mpeg4video_parser.c
  • [DH] libavcodec/mpeg4videodec.c

• avcodec/mpeg4videodec : Move use_intra_dc_vlc to stack, fix data race

  26 janvier 2022, par Andreas Rheinhardt

  avcodec/mpeg4videodec : Move use_intra_dc_vlc to stack, fix data race
  use_intra_dc_vlc is currently kept in sync between frame threads
  
  in mpeg4_update_thread_context(), yet it is set when decoding
  
  blocks, i.e. after ff_thread_finish_setup(). This is a data race
  
  and therefore undefined behaviour.
  This race can be fixed easily by moving the variable from the context
  
  to the stack : use_intra_dc_vlc is only read in
  
  mpeg4_decode_block() and only if one is decoding an intra block.
  
  There are three callsites for this function : One in
  
  mpeg4_decode_partitioned_mb() which always sets use_intra_dc_vlc
  
  before the call and two in mpeg4_decode_mb(). One of these callsites
  
  is for intra blocks and use_intra_dc_vlc is set before it ;
  
  the last callsite is for non-intra blocks, where use_intra_dc_vlc
  
  is ignored. So if it is used, it always uses a new value and can
  
  therefore be moved to the stack.
  The above also explains why this data race did not lead to
  
  FATE-test failures.
  Signed-off-by : Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
  

  • [DH] libavcodec/mpeg4video.h
  • [DH] libavcodec/mpeg4videodec.c

• avcodec/mpegpicture : Don't copy unnecessarily, fix race

  8 août 2022, par Andreas Rheinhardt

  avcodec/mpegpicture : Don't copy unnecessarily, fix race
  mpegvideo uses an array of Pictures and when it is done with using
  
  them, it only unreferences them incompletely : Some buffers are kept
  
  so that they can be reused lateron if the same slot in the Picture
  
  array is reused, making this a sort of a bufferpool.
  
  (Basically, a Picture is considered used if the AVFrame's buf is set.)
  
  Yet given that other pieces of the decoder may have a reference to
  
  these buffers, they need not be writable and are made writable using
  
  av_buffer_make_writable() when preparing a new Picture. This involves
  
  reading the buffer's data, although the old content of the buffer
  
  need not be retained.
  Worse, this read can be racy, because the buffer can be used by another
  
  thread at the same time. This happens for Real Video 3 and 4.
  This commit fixes this race by no longer copying the data ;
  
  instead the old buffer is replaced by a new, zero-allocated buffer.
  (Here are the details of what happens with three or more decoding threads
  
  when decoding rv30.rm from the FATE-suite as happens in the rv30 test :
  
  The first decoding thread uses the first slot of its picture array
  
  to store its current pic ; update_thread_context copies this for the
  
  second thread that decodes a P-frame. It uses the second slot in its
  
  Picture array to store its P-frame. This arrangement is then copied
  
  to the third decode thread, which decodes a B-frame. It uses the third
  
  slot in its Picture array for its current frame.
  
  update_thread_context copies this to the next thread. It unreferences
  
  the third slot containing the other B-frame and then it reuses this
  
  slot for its current frame. Because the pic array slots are only
  
  incompletely unreferenced, the buffers of the previous B-frame are
  
  still in there and they are not writable ; in fact the previous
  
  thread is concurrently writing to them, causing races when making
  
  the buffer writable.)
  Signed-off-by : Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
  

  • [DH] libavcodec/mpegpicture.c