1. 08 Oct, 2019 34 commits
  2. 07 Oct, 2019 6 commits
    • Andreas Rheinhardt's avatar
      1d54309c
    • Andreas Rheinhardt's avatar
    • Andreas Rheinhardt's avatar
      avcodec/flac_parser: Don't modify size of the input buffer · 87b30f8a
      Andreas Rheinhardt authored
      When flushing, MAX_FRAME_HEADER_SIZE bytes (always zero) are supposed to
      be written to the fifo buffer in order to be able to check the rest of
      the buffer for frame headers. It was intended to write these by writing
      a small buffer of size MAX_FRAME_HEADER_SIZE to the buffer. But the way
      it was actually done ensured that this did not happen:
      
      First, it would be checked whether the size of the input buffer was zero,
      in which case it buf_size would be set to MAX_FRAME_HEADER_SIZE and
      read_end would be set to indicate that MAX_FRAME_HEADER_SIZE bytes need
      to be written. Then it would be made sure that there is enough space in
      the fifo for the data to be written. Afterwards the data is written. The
      check used here is for whether buf_size is zero or not. But if it was
      zero initially, it is MAX_FRAME_HEADER_SIZE now, so that not the
      designated buffer for writing MAX_FRAME_HEADER_SIZE is written; instead
      the padded buffer (from the stack of av_parser_parse2()) is used. This
      works because AV_INPUT_BUFFER_PADDING_SIZE >= MAX_FRAME_HEADER_SIZE.
      Lateron, buf_size is set to zero again.
      
      Given that since 7edbd536, the actual amount of data read is no longer
      automatically equal to buf_size, it is completely unnecessary to modify
      buf_size at all. Moreover, modifying it is dangerous: Some allocations
      can fail and because buf_size is never reset to zero in this codepath,
      the parser might return a value > 0 on flushing.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      87b30f8a
    • Andreas Rheinhardt's avatar
      avcodec/flac_parser: Remove superfluous checks · a1701e75
      Andreas Rheinhardt authored
      For a parser, the input buffer is always != NULL: In case of flushing,
      the indicated size of the input buffer will be zero and the input buffer
      will point to a zeroed buffer of size 0 + AV_INPUT_BUFFER_PADDING.
      Therefore one does not need to check for whether said buffer is NULL or
      not.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      a1701e75
    • Andreas Rheinhardt's avatar
      avcodec/flac_parser: Fix number of buffered headers · 047a6d39
      Andreas Rheinhardt authored
      Only decrement the number of buffered headers if a header has actually
      been freed.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      047a6d39
    • Andreas Rheinhardt's avatar
      avcodec/flac_parser: Fix off-by-one error · e5e5be4c
      Andreas Rheinhardt authored
      The flac parser uses a fifo to buffer its data. Consequently, when
      searching for sync codes of flac packets, one needs to take care of
      the possibility of wraparound. This is done by using an optimized start
      code search that works on each of the continuous buffers separately and
      by explicitly checking whether the last pre-wrap byte and the first
      post-wrap byte constitute a valid sync code.
      
      Moreover, the last MAX_FRAME_HEADER_SIZE - 1 bytes ought not to be searched
      for (the start of) a sync code because a header that might be found in this
      region might not be completely available. These bytes ought to be searched
      lateron when more data is available or when flushing.
      
      Unfortunately there was an off-by-one error in the calculation of the
      length to search of the post-wrap buffer: It was too large, because the
      calculation was based on the amount of bytes available in the fifo from
      the last pre-wrap byte onwards. This meant that a header might be
      parsed twice (once prematurely and once regularly when more data is
      available); it could also mean that an invalid header will be treated as
      valid (namely if the length of said invalid header is
      MAX_FRAME_HEADER_SIZE and the invalid byte that will be treated as the
      last byte of this potential header happens to be the right CRC-8).
      
      Should a header be parsed twice, the second instance will be the best child
      of the first instance; the first instance's score will be
      FLAC_HEADER_BASE_SCORE - FLAC_HEADER_CHANGED_PENALTY ( = 3) higher than
      the second instance's score. So the frame belonging to the first
      instance will be output and it will be done as a zero length frame (the
      difference of the header's offset and the child's offset). This has
      serious consequences when flushing, as returning a zero length buffer
      signals to the caller that no more data will be output; consequently the
      last frames not yet output will be dropped.
      
      Furthermore, a "sample/frame number mismatch in adjacent frames" warning
      got output when returning the zero-length frame belonging to the first
      header, because the child's sample/frame number of course didn't match
      the expected sample frame/number given its parent.
      
      filter/hdcd-mix.flac from the FATE-suite was affected by this (the last
      frame was omitted) which is the reason why several FATE-tests needed to
      be updated.
      
      Fixes ticket #5937.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      e5e5be4c