| Message ID | 1505158907-33193-1-git-send-email-rsbultje@gmail.com |
|---|---|
| State | Accepted |
| Commit | 4ce99e96d6115ccd1fc82f826d4c628240ef53ed |
| Headers | show |
On 9/11/2017 4:41 PM, Ronald S. Bultje wrote: > --- > libavcodec/vp9.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > index f626f81..66ccb6c 100644 > --- a/libavcodec/vp9.c > +++ b/libavcodec/vp9.c > @@ -1603,7 +1603,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > if (avctx->active_thread_type == FF_THREAD_SLICE) { > int tile_row, tile_col; > > - assert(!pass); > + av_assert1(!s->pass); avassert.h says av_assert1() * assert() equivalent, that does not lie in speed critical code. * These asserts() thus can be enabled without fearing speed loss. av_assert2() * assert() equivalent, that does lie in speed critical code. Since this is in a loop inside vp9_decode_frame() i think the latter is more correct. But patch LGTM either way. > > for (tile_row = 0; tile_row < s->s.h.tiling.tile_rows; tile_row++) { > for (tile_col = 0; tile_col < s->s.h.tiling.tile_cols; tile_col++) { >
Hi, On Mon, Sep 11, 2017 at 3:58 PM, James Almer <jamrial@gmail.com> wrote: > On 9/11/2017 4:41 PM, Ronald S. Bultje wrote: > > --- > > libavcodec/vp9.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > > index f626f81..66ccb6c 100644 > > --- a/libavcodec/vp9.c > > +++ b/libavcodec/vp9.c > > @@ -1603,7 +1603,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > > if (avctx->active_thread_type == FF_THREAD_SLICE) { > > int tile_row, tile_col; > > > > - assert(!pass); > > + av_assert1(!s->pass); > > avassert.h says > > av_assert1() > * assert() equivalent, that does not lie in speed critical code. > * These asserts() thus can be enabled without fearing speed loss. > > av_assert2() > * assert() equivalent, that does lie in speed critical code. > > Since this is in a loop inside vp9_decode_frame() i think the latter is > more correct. But patch LGTM either way. As discussed on IRC, this is not a per-block loop, but rather a "2-pass" decoding loop (i.e. the loop executes either once or twice), and this particular piece of code only executes in 1-pass decoding mode (so it is guaranteed to execute once per frame), so we agreed av_assert1 is fine. Pushed. Ronald
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c index f626f81..66ccb6c 100644 --- a/libavcodec/vp9.c +++ b/libavcodec/vp9.c @@ -1603,7 +1603,7 @@ FF_ENABLE_DEPRECATION_WARNINGS if (avctx->active_thread_type == FF_THREAD_SLICE) { int tile_row, tile_col; - assert(!pass); + av_assert1(!s->pass); for (tile_row = 0; tile_row < s->s.h.tiling.tile_rows; tile_row++) { for (tile_col = 0; tile_col < s->s.h.tiling.tile_cols; tile_col++) {