On Tue, May 3, 2016 at 12:00 PM, Hendrik Leppkes <[email protected]> wrote: > On Tue, May 3, 2016 at 5:50 PM, Vittorio Giovara > <[email protected]> wrote: >> On Tue, May 3, 2016 at 7:25 AM, 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. >> >> IMO Anton is right wrt common/uncommon, but Kieran is right for the >> trailing number being confusing. >> I would propose bitstream_uintread and bitstream_longread which would >> make it perfectly clear the maximum length of the read value. > > int and long have the same length on many systems. :p
Right, but that's nothing that the documentation can fix, and imo less confusing than trailing numbers. Feel free to bikeshed on the names though. -- Vittorio _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
