On Wed, May 15, 2019 at 7:01 AM Hendrik Leppkes <h.lepp...@gmail.com> wrote: > > 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. > I agree this part > > > > 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. > If assert0 or assert1 lead to performance drop, we need some profiling data, then try to fix it. > - 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".