Quoting Kieran Kunhya (2016-05-03 13:54:49) > On Tue, 3 May 2016 at 12:26 Anton Khirnov <[email protected]> wrote: > > > Quoting Kieran Kunhya (2016-05-03 11:33:42) > > > On Tue, 3 May 2016 at 07:43 Luca Barbato <[email protected]> wrote: > > > > > > > On 03/05/16 15:34, Kieran Kunhya wrote: > > > > > I disagree, the old names are relatively clear. Whilst I think the > > speed > > > > > improvements in this patch are great, the function names like > > > > bitstream_read_32 > > > > > are really confusing. IMO adding a number suffix should be the > > exception > > > > > rather than the norm (i.e when reading large numbers of bits). > > > > > > > > The past code shown that not having the number of bits would make > > people > > > > assume such functions work for the wrong range. > > > > > > > > The new functions support a larger range BUT I had bitten once too many > > > > to consider using _long for the 63 bits variant. > > > > > > > > Yes but reading > 32 bits isn't very common so it should be treated as > > the > > > special case. > > > All these _32s make things very very unreadable. I want the unusual cases > > > to have special suffixes. > > > > I'm not buying those "common" vs "uncommon" arguments. Experience shows > > that people get it wrong all the time with the current code, so the new > > API should make it very explicit what limitation does each variant have. > > > > There are asserts for this exact reason. It's totally unnecessary to remind > the reader/writer on every use of the function what the bounds are.
Asserts are either disabled, meaning they don't actually check anything, or you have an extra branch meaning a likely slowdown. I much prefer having a slightly longer, but more descriptive name. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
