konablend / mkvtoolnix Goto Github PK

Clang generates and activates precompiled headers in a different manner than gcc. Adding support entails:

• flag -emit-pch to generate .pch binary (similar to .gch)
• flag -include-pch (on a per-source-file basis) to activate

• dts_xll.cpp: Please use 2 spaces for indentation everywhere (the format_each… functions use 4).
  3597018 source code formatting
• The format_each… template functions look like they should be placed in their own header file in src/common as that functionality may very well be useful for other code, too.
• Same applies to the result_guard class which I like.
  854041f commonify format_each and result_guard
  8c10b40 move format.h and sbrm.h out of common.h
• Please name classes with a _c postfix for consistency with the rest of the program (result_guard → result_guard_c).
  1a1241e rename result_guard -> result_guard_c
• The CRC16 calculation should really not require a new function. Usually all you have to do is figure out the initial value, the final value to XOR, the endianness, and then the routines from src/common/checksums* should be usable. See input/r_mpeg_ts.cpp for an example: src/input/r_mpeg_ts.cpp lines 797 ff »mpeg_ts_reader_c::calculate_crc(…)«
  ae4e5a5 use mtx::checksum::calculate_as_uint
• About the failing tests: I'll investigate them.
  55928ec fix regression: set core_sampling_frequency
  0acf143 fix regression: asset.xll_sync_preset
• The s_downmix_table: we may have to make that translatable; probably by using the translatable_string_c class and the YT() macro.
  f13df4e make downmix_table_t translatable
• »using xll_header = … shared_ptr…« ← Again consistency: please use a postfix of _cptr for everything that's a shared_ptr (xll_header_cptr) (cptr = counted_ptr = my own template class I used to use back in the day before std::shared_ptr was available)
  6991bfd append _cptr to shared_ptr typenames
• A question: you've copied my usualy copyright header to dts_xll.cpp. Would you be OK with licensing your changes under »GPLv2 oder any later version«, too, instead of just »GPLv2 only«? Reason I'm asking is:
  Currently MKVToolNix is licensed under GPLv2 only. However, there are libraries that are (L)GPLv3 that I might like to use in the future, and combining »GPLv2 only« with »(L)GPLv3« is not possible.
  (What I'm thinking about is Qt, BTW, which are opening up several of their formerly proprietary additions under the (L)GPLv3, and I might want to use some of those in the future).
  1d5a664 modify copyright
  4990d43 format copyright
  f39bb5e modify copyright

• test/run.rb warning: sysctl: unknown oid 'hw.availcpu'
• responsible code here