Message ID | E1cQDcM-0007Mm-8N@pannekake.samfundet.no |
---|---|
State | Superseded |
Headers | show |
On 1/8/2017 10:14 AM, Steinar H. Gunderson wrote: > Allows more codecs than mpeg12video to make use of it. Rename to av_bitswap_32() > because it's a public symbol now. mathops.h is not a public header. You can keep the bitswap_32() name or use ff_bitswap_32() instead. Also, when moving code in general we expect a patch that makes use of said code as part of a set. > --- > libavcodec/bitstream.c | 14 +++----------- > libavcodec/mathops.h | 8 ++++++++ > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c > index 6c8dca1d85..5d37430540 100644 > --- a/libavcodec/bitstream.c > +++ b/libavcodec/bitstream.c > @@ -128,14 +128,6 @@ static int alloc_table(VLC *vlc, int size, int use_static) > return index; > } > > -static av_always_inline uint32_t bitswap_32(uint32_t x) > -{ > - return (uint32_t)ff_reverse[ x & 0xFF] << 24 | > - (uint32_t)ff_reverse[(x >> 8) & 0xFF] << 16 | > - (uint32_t)ff_reverse[(x >> 16) & 0xFF] << 8 | > - (uint32_t)ff_reverse[ x >> 24]; > -} > - > typedef struct VLCcode { > uint8_t bits; > uint16_t symbol; > @@ -192,7 +184,7 @@ static int build_table(VLC *vlc, int table_nb_bits, int nb_codes, > nb = 1 << (table_nb_bits - n); > inc = 1; > if (flags & INIT_VLC_LE) { > - j = bitswap_32(code); > + j = av_bitswap_32(code); > inc = 1 << n; > } > for (k = 0; k < nb; k++) { > @@ -225,7 +217,7 @@ static int build_table(VLC *vlc, int table_nb_bits, int nb_codes, > subtable_bits = FFMAX(subtable_bits, n); > } > subtable_bits = FFMIN(subtable_bits, table_nb_bits); > - j = (flags & INIT_VLC_LE) ? bitswap_32(code_prefix) >> (32 - table_nb_bits) : code_prefix; > + j = (flags & INIT_VLC_LE) ? av_bitswap_32(code_prefix) >> (32 - table_nb_bits) : code_prefix; > table[j][1] = -subtable_bits; > ff_dlog(NULL, "%4x: n=%d (subtable)\n", > j, codes[i].bits + table_nb_bits); > @@ -325,7 +317,7 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int nb_codes, > return -1; \ > } \ > if (flags & INIT_VLC_LE) \ > - buf[j].code = bitswap_32(buf[j].code); \ > + buf[j].code = av_bitswap_32(buf[j].code); \ > else \ > buf[j].code <<= 32 - buf[j].bits; \ > if (symbols) \ > diff --git a/libavcodec/mathops.h b/libavcodec/mathops.h > index 5168dc2ce0..65dc8f21f1 100644 > --- a/libavcodec/mathops.h > +++ b/libavcodec/mathops.h > @@ -249,4 +249,12 @@ static inline int8_t ff_u8_to_s8(uint8_t a) > return b.s8; > } > > +static av_always_inline uint32_t av_bitswap_32(uint32_t x) > +{ > + return (uint32_t)ff_reverse[ x & 0xFF] << 24 | > + (uint32_t)ff_reverse[(x >> 8) & 0xFF] << 16 | > + (uint32_t)ff_reverse[(x >> 16) & 0xFF] << 8 | > + (uint32_t)ff_reverse[ x >> 24]; > +} > + > #endif /* AVCODEC_MATHOPS_H */ >
On Sun, Jan 08, 2017 at 10:52:52AM -0300, James Almer wrote: > Also, when moving code in general we expect a patch that makes use of said > code as part of a set. I don't understand. I send a patch where I both make the move and add a new use. Then I get told to split them into two separate patches and do so; now I get told that they need to come together? :-) /* Steinar */
On 1/8/2017 10:55 AM, Steinar H. Gunderson wrote: > On Sun, Jan 08, 2017 at 10:52:52AM -0300, James Almer wrote: >> Also, when moving code in general we expect a patch that makes use of said >> code as part of a set. > > I don't understand. I send a patch where I both make the move and add a > new use. Then I get told to split them into two separate patches and do so; > now I get told that they need to come together? :-) > > /* Steinar */ My bad, I see now this is patch 1 of 2. I didn't notice the second one since it's on a separate thread instead of as a reply to this one. Hence why i mentioned this first patch alone is not of much use. Disregard that part then. Just address the name of the function.
On Sun, Jan 08, 2017 at 11:01:14AM -0300, James Almer wrote:
> Disregard that part then. Just address the name of the function.
Will do. Waiting with sending of a new patch to see if there are more
comments coming that I can address in the same batch.
/* Steinar */
diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c index 6c8dca1d85..5d37430540 100644 --- a/libavcodec/bitstream.c +++ b/libavcodec/bitstream.c @@ -128,14 +128,6 @@ static int alloc_table(VLC *vlc, int size, int use_static) return index; } -static av_always_inline uint32_t bitswap_32(uint32_t x) -{ - return (uint32_t)ff_reverse[ x & 0xFF] << 24 | - (uint32_t)ff_reverse[(x >> 8) & 0xFF] << 16 | - (uint32_t)ff_reverse[(x >> 16) & 0xFF] << 8 | - (uint32_t)ff_reverse[ x >> 24]; -} - typedef struct VLCcode { uint8_t bits; uint16_t symbol; @@ -192,7 +184,7 @@ static int build_table(VLC *vlc, int table_nb_bits, int nb_codes, nb = 1 << (table_nb_bits - n); inc = 1; if (flags & INIT_VLC_LE) { - j = bitswap_32(code); + j = av_bitswap_32(code); inc = 1 << n; } for (k = 0; k < nb; k++) { @@ -225,7 +217,7 @@ static int build_table(VLC *vlc, int table_nb_bits, int nb_codes, subtable_bits = FFMAX(subtable_bits, n); } subtable_bits = FFMIN(subtable_bits, table_nb_bits); - j = (flags & INIT_VLC_LE) ? bitswap_32(code_prefix) >> (32 - table_nb_bits) : code_prefix; + j = (flags & INIT_VLC_LE) ? av_bitswap_32(code_prefix) >> (32 - table_nb_bits) : code_prefix; table[j][1] = -subtable_bits; ff_dlog(NULL, "%4x: n=%d (subtable)\n", j, codes[i].bits + table_nb_bits); @@ -325,7 +317,7 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int nb_codes, return -1; \ } \ if (flags & INIT_VLC_LE) \ - buf[j].code = bitswap_32(buf[j].code); \ + buf[j].code = av_bitswap_32(buf[j].code); \ else \ buf[j].code <<= 32 - buf[j].bits; \ if (symbols) \ diff --git a/libavcodec/mathops.h b/libavcodec/mathops.h index 5168dc2ce0..65dc8f21f1 100644 --- a/libavcodec/mathops.h +++ b/libavcodec/mathops.h @@ -249,4 +249,12 @@ static inline int8_t ff_u8_to_s8(uint8_t a) return b.s8; } +static av_always_inline uint32_t av_bitswap_32(uint32_t x) +{ + return (uint32_t)ff_reverse[ x & 0xFF] << 24 | + (uint32_t)ff_reverse[(x >> 8) & 0xFF] << 16 | + (uint32_t)ff_reverse[(x >> 16) & 0xFF] << 8 | + (uint32_t)ff_reverse[ x >> 24]; +} + #endif /* AVCODEC_MATHOPS_H */