On Tue, May 14, 2019 at 11:25 PM Adam Richter <adamricht...@gmail.com> wrote: > > Consider, for example, if you agree that columnization makes this range check > more recognizable in a glance and makes it easier to spot what the bounds are > (the sixteen space indentation is taken from the code in which it appeared): > > av_assert0(par->bits_per_coded_sample >= 0 && > par->bits_per_coded_sample <= 8); > > ...vs... > > av_assert0(par->bits_per_coded_sample >= 0); > av_assert0(par->bits_per_coded_sample <= 8); > > A possible counter-argument to this might be that, in a long sequence > of assertions, can be informative to group related assertions > together, which I think is true, but it is possible to get this other > means, such as by using blank lines to separate express such grouping. > > So, Mark, if you decide you are OK with my complete patches, I encourage > you to let me know. Otherwise, if there are any of those changes which you > are OK with, I would like to just to to get those merged. Finally, if you > would > rather see none of those changes merged (one one to split the assertions in > libavformat and one was for everywhere else in ffmpeg), please let me know > about that too, in which case, if no one advocates for their > inclusion, I'll drop > my proposal to merge these changes. >
Unfortunately I have to agree with Mark. asserst that check the same value or extremely closely related values should stay combined. > > Also after this, I may take a look at adding a branch hint to the av_assertN > macros if nobody objects. > Please don't, we don't do branch hints anywhere, and this is a bad place to start. If an assert is truely performance sensitive (as in, it makes a measurable difference), it should probably be moved from assert0 to assert1, and as such is only enabled in testing builds. - Hendrik _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
