On Mon, Oct 12, 2015 at 12:37 AM, James Almer <jamr...@gmail.com> wrote: > On 10/11/2015 10:55 PM, Ganesh Ajjanagadde wrote: >> It has already been demonstrated that the de Bruijn method has benefits >> over the current implementation: commit >> 971d12b7f9d7be3ca8eb98e6c04ed521f83cbd3c. >> That commit implemented it for long long, this extends it to the 32 bit >> version. >> >> The function is renamed from ff_ctz to ff_ctz32 since it crucially >> depends on the 32 bit width of its argument. This is not an issue, as the >> only usage in avcodec/flacenc uses an int32_t anyway. > > I personally don't think the renaming is needed, for that matter. The > function takes an int as argument, and as far as ffmpeg supported arches > go those are 32 bits. > If you really want to be sure, just add a comment that the argument > absolutely needs to be 32 bits and that should be enough.
This I don't understand. Why not make the function self documenting when we achieve it with zero penalty? I consider it ridiculous to add a comment when the name tells what it does better and more succinctly. We could add both, but I do not want to do that as generally FFmpeg favors a slightly terse style common to many C projects. Thus, if adding the width, it should be to the function name, not to a comment. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
