1. 12 Apr, 2020 1 commit
    • Andreas Rheinhardt's avatar
      avcodec/cbs: Avoid leaving the ... out in calls to variadic macros · 14dd0a90
      Andreas Rheinhardt authored
      According to C99, there has to be at least one argument for every ...
      in a variadic function-like macro. In practice most (all?) compilers also
      allow to leave it completely out, but it is nevertheless required: In a
      variadic macro "there shall be more arguments in the invocation than there
      are parameters in the macro definition (excluding the ...)." (C99,
      6.10.3.4).
      
      CBS (not the framework itself, but the macros used in the
      cbs_*_syntax_template.c files) relies on the compiler allowing to leave
      a variadic macro argument out. This leads to warnings when compiling in
      -pedantic mode, e.g. "warning: must specify at least one argument for
      '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]"
      from Clang.
      
      Most of these warnings can be easily avoided: The syntax_templates
      mostly contain helper macros that expand to more complex variadic macros
      and these helper macros often omit an argument for the .... Modifying
      them to always expand to complex macros with an empty argument for the
      ... at the end fixes most of these warnings: The number of warnings went
      down from 400 to 0 for cbs_av1, from 1114 to 32 for cbs_h2645, from 38 to
      0 for cbs_jpeg, from 166 to 0 for cbs_mpeg2 and from 110 to 8 for cbs_vp9.
      
      These eight remaining warnings for cbs_vp9 have been fixed by switching
      to another macro in cbs_vp9_syntax_template: The fixed values for the
      sync bytes as well as the trailing bits for byte-alignment are now read
      via the fixed() macro (this also adds a check to ensure that trailing
      bits are indeed zero as they have to be).
      Reviewed-by: 's avatarMark Thompson <sw@jkqxz.net>
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      14dd0a90
  2. 09 Feb, 2020 1 commit
  3. 17 Nov, 2019 2 commits
    • Andreas Rheinhardt's avatar
      avcodec/cbs: Fix potential overflow · cda3e8ca
      Andreas Rheinhardt authored
      The number of bits in a PutBitContext must fit into an int, yet nothing
      guaranteed the size argument cbs_write_unit_data() uses in init_put_bits()
      to be in the range 0..INT_MAX / 8. This has been changed.
      
      Furthermore, the check 8 * data_size > data_bit_start that there is
      data beyond the initial padding when writing mpeg2 or H.264/5 slices
      could also overflow, so divide it by 8 to get an equivalent check
      without this problem.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      cda3e8ca
    • Andreas Rheinhardt's avatar
      avcodec/cbs: Factor out common code for writing units · 7c92eaac
      Andreas Rheinhardt authored
      All cbs-functions to write units share a common pattern:
      1. They check whether they have a write buffer (that is used to store
      the unit's data until the needed size becomes known after writing the
      unit when a dedicated buffer will be allocated).
      2. They use this buffer for a PutBitContext.
      3. The (codec-specific) writing takes place through the PutBitContext.
      4. The return value is checked. AVERROR(ENOSPC) here always indicates
      that the buffer was too small and leads to a reallocation of said
      buffer.
      5. The final buffer will be allocated and the data copied.
      
      This commit factors this common code out in a single function in cbs.c.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      7c92eaac
  4. 29 Jul, 2019 4 commits
    • Andreas Rheinhardt's avatar
      cbs_mpeg2: Fix parsing the last unit · fd93d5ef
      Andreas Rheinhardt authored
      There is one way to find out if avpriv_find_start_code has found a start
      code or not: One has to check whether the state variable contains a
      start code, i.e. whether the three most significant bytes are 0x00 00 01.
      Checking for whether the return value is the end of the designated
      buffer is not enough: If the last four bytes constitute a start code,
      the return value is also the end of the buffer. This happens with
      sequence_end_codes which have been ignored for exactly this reason,
      although e.g. all three files used for fate tests of cbs_mpeg2 contain
      sequence_end_codes.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      fd93d5ef
    • Andreas Rheinhardt's avatar
      cbs_mpeg2: Rearrange start code search · 276b21a5
      Andreas Rheinhardt authored
      1. Currently, cbs_mpeg2_split_fragment uses essentially three variables
      to hold the start code values found by avpriv_find_start_code. By
      rearranging the code, one of them can be omitted.
      2. The return value of avpriv_find_start_code points to the byte after
      the byte containing the start code identifier (or to the byte after the
      last byte of the fragment's data if no start code was found), but
      cbs_mpeg2_split_fragment needs to work with the pointer to the byte
      containing the start code identifier; it already did this, but in a
      clumsy way. This has been changed.
      3. Also use the correct type for the variable holding the
      CodedBitstreamUnitType.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      276b21a5
    • Andreas Rheinhardt's avatar
      cbs_mpeg2: Decompose Sequence End · 0e66e1b6
      Andreas Rheinhardt authored
      Sequence End units (or actually, sequence_end_codes) have up until now
      not been decomposed; in fact due to a bug in cbs_mpeg2_split_fragment they
      have mostly been treated as part of the preceding unit. So implement
      decomposing them as preparation for fixing said bug.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      0e66e1b6
    • Andreas Rheinhardt's avatar
      cbs: Don't set AVBuffer's opaque · 4e7e30bb
      Andreas Rheinhardt authored
      cbs is currently inconsistent regarding the opaque field that can be
      used as a special argument to av_buffer_create in order to be used
      during freeing the buffer: ff_cbs_alloc_unit_content and all the free
      functions used name this parameter as if it should contain a pointer to
      the unit whose content is about to be created; but both
      ff_cbs_alloc_unit_content as well as ff_cbs_h264_add_sei_message
      actually use a pointer to the CodedBitstreamContext as opaque. It should
      actually be neither, because it is unneeded (as is evidenced by the fact
      that none of the free functions use this pointer at all) and because it
      ties the unit's content to the lifetime of other objects, although a
      refcounted buffer is supposed to have its own lifetime that only ends
      when its reference count reaches zero. This problem manifests itself in
      the pointer becoming dangling.
      The pointer to the unit can become dangling if another unit is added to
      the fragment later as happens in the bitstream filters; in this case,
      the pointer can point to the wrong unit (if the fragment's unit array
      needn't be relocated) or it can point to where the array was earlier.
      It can also become dangling if the unit's content is meant to survive
      the resetting of the fragment it was originally read with. This applies
      to the extradata of H.264 and HEVC.
      The pointer to the context can become dangling if the context is closed
      before the content is freed. Although this doesn't seem to happen right
      now, it could happen, in particular if one uses different
      CodedBitstreamContexts for in- and output.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      4e7e30bb
  5. 27 Jul, 2019 3 commits
    • Andreas Rheinhardt's avatar
      cbs_mpeg2: Fix parsing of picture and slice headers · d9182f04
      Andreas Rheinhardt authored
      1. The extra information in slice headers was parsed incorrectly:
      In the first reading pass to derive the length of the extra information,
      one should look at bits n, n + 9, n + 18, ... and check whether they
      equal one (further extra information) or zero (end of extra information),
      but instead bits n, n + 8, n + 16, ... were inspected. The second pass
      of reading (where the length is already known and the bytes between the
      length-determining bits are copied into a buffer) did not record what
      was in bits n, n + 9, n + 18, ..., presuming they equal one. And during
      writing, the bytes in the buffer are interleaved with set bits and
      written. This means that if the detected length of the extra information
      was greater than the real length, the output was corrupted. Fortunately
      no sample is known that made use of this mechanism: The extra information
      in slices is still marked as reserved in the specifications. cbs_mpeg2
      is now ready in case this changes.
      
      2. Furthermore, the buffer is now padded and slightly different, but
      very similar code for reading resp. writing has been replaced by code
      used for both. This was made possible by a new macro, the equivalent
      to cbs_h2645's fixed().
      
      3. These changes also made it possible to remove the extra_bit_slice
      element from the MPEG2RawSliceHeader structure. Said element was always
      zero except when the detected length of the extra information was less
      than the real length.
      
      4. The extra information in picture headers (which uses essentially the
      same syntax as the extra information in slice headers) has simply been
      forgotten. This meant that if this extra information was present, it was
      discarded during reading; and unfortunately writing created invalid
      bitstreams in this case (an extra_bit_picture - the last set bit of the
      whole unit - indicated that there would be a further byte of data,
      although the output didn't contain said data).
      
      This has been fixed; both types of extra information are now parsed via
      the same code and essentially passed through.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      d9182f04
    • Andreas Rheinhardt's avatar
      cbs: Remove useless initializations · b71a0367
      Andreas Rheinhardt authored
      Up until now, a temporary variable was used and initialized every time a
      value was read in CBS; if reading turned out to be successfull, this
      value was overwritten (without having ever been looked at) with the
      value read if reading was successfull; on failure the variable wasn't
      touched either. Therefore these initializations can be and have been
      removed.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      b71a0367
    • Andreas Rheinhardt's avatar
      mpeg2_metadata, cbs_mpeg2: Fix handling of colour_description · c2a91645
      Andreas Rheinhardt authored
      If a sequence display extension is read with colour_description equal to
      zero, but a user wants to add one or more of the colour_description
      elements, then the colour_description elements the user did not explicitly
      request to be set are set to zero and not to the value equal to
      unknown/unspecified (namely 2). A value of zero is not only inappropriate,
      but explicitly forbidden. This is fixed by inferring the right default
      values during the reading process if the elements are absent; moreover,
      changing any of the colour_description elements to zero is now no longer
      possible.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      c2a91645
  6. 07 Jun, 2019 1 commit
  7. 28 May, 2019 4 commits
    • Andreas Rheinhardt's avatar
      cbs_mpeg2: Correct error codes · 1759a9e5
      Andreas Rheinhardt authored
      Up until now, things that are merely unsupported by cbs_mpeg2 have been
      declared to be invalid input. This has been changed.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      1759a9e5
    • Andreas Rheinhardt's avatar
      cbs_mpeg2: Fix storage type for frame_centre_*_offset · de588038
      Andreas Rheinhardt authored
      The frame_centre_horizontal/vertical_offset values contained in picture
      display extensions are actually signed values (i.e. it is possible to
      indicate that the display device should add black bars/pillars).
      
      The files sony-ct3.bs and tcela-6.bits (which are both used in fate
      tests for mpeg2_metadata) contain picture display extensions; the former
      even contains a negative frame_centre_vertical_offset. Fortunately, the
      old code did not damage the picture display extensions when one did a
      cycle of reading and writing. For the same reason the fate tests needn't
      be updated either.
      
      Furthermore these fields now use the trace output for matrices.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      de588038
    • Andreas Rheinhardt's avatar
      cbs_mpeg2: Improve checks for invalid values · 9c3f2a88
      Andreas Rheinhardt authored
      MPEG-2 contains several elements that mustn't be zero according to the
      specifications: horizontal/vertical_size_value, aspect_ratio_information,
      frame_rate_code, the quantiser matrices, the colour_description
      elements, picture_coding_type, the f_code[r][s] values and
      quantiser_scale_code. It is now checked that the invalid values don't
      occur.
      
      The colour_description elements are treated specially in this regard:
      Given that there are files in the wild which use illegal values for the
      colour_description elements (some of them created by mpeg2_metadata),
      they will be corrected to the value meaning "unknown" (namely 2) during
      reading. This has been done in such a way that trace_headers will
      nevertheless report the original value, together with a message about
      the fixup.
      
      Furthermore, the trace_headers output of user_data has been beautified.
      Signed-off-by: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
      9c3f2a88
    • Andreas Rheinhardt's avatar
  8. 11 Nov, 2018 1 commit
  9. 02 May, 2018 1 commit
  10. 30 Apr, 2018 1 commit
  11. 26 Apr, 2018 1 commit
  12. 25 Apr, 2018 1 commit
  13. 05 Mar, 2018 1 commit
  14. 20 Feb, 2018 2 commits
    • Mark Thompson's avatar
      cbs: Refcount all the things! · ce5870a3
      Mark Thompson authored
      This makes it easier for users of the CBS API to get alloc/free right -
      all subelements use the buffer API so that it's clear how to free them.
      It also allows eliding some redundant copies: the packet -> fragment copy
      disappears after this change if the input packet is refcounted, and more
      codec-specific cases are now possible (but not included in this patch).
      ce5870a3
    • Mark Thompson's avatar
      cbs: Add an explicit type for coded bitstream unit types · 1d12a545
      Mark Thompson authored
      Also fix conversion specifiers used for the unit type.
      1d12a545
  15. 17 Dec, 2017 1 commit
  16. 02 Dec, 2017 2 commits
  17. 24 Oct, 2017 2 commits
  18. 17 Oct, 2017 1 commit
  19. 12 Sep, 2017 2 commits
  20. 20 Aug, 2017 1 commit