[FFmpeg-cvslog] r17362 - trunk/libavcodec/mpegvideo_xvmc.c

[Diego Biurrun]

On Mon, Feb 16, 2009 at 09:50:32AM +0200, Ivan Kalvachev wrote:
> On 2/16/09, diego <subversion at mplayerhq.hu> wrote:
> >
> > Log:
> > Replace two asserts by checks and error messages.
> >
> > --- trunk/libavcodec/mpegvideo_xvmc.c	Mon Feb 16 02:59:51 2009	(r17361)
> > +++ trunk/libavcodec/mpegvideo_xvmc.c	Mon Feb 16 03:02:49 2009	(r17362)
> > @@ -312,11 +312,14 @@ void ff_xvmc_decode_mb(MpegEncContext *s
> >
> > -    assert(render->filled_mv_blocks_num     <= render->allocated_mv_blocks);
> > -    assert(render->next_free_data_block_num <= render->allocated_data_blocks);
> > -    /* The above conditions should not be able to fail as long as this function
> > -     * is used and the following 'if ()' automatically calls a callback to free
> > -     * blocks. */
> > +
> > +    if (render->filled_mv_blocks_num > render->allocated_mv_blocks)
> > +        av_log(s->avctx, AV_LOG_ERROR,
> > +               "Not enough space to store mv blocks allocated.\n");
> > +
> > +    if (render->next_free_data_block_num > render->allocated_data_blocks)
> > +        av_log(s->avctx, AV_LOG_ERROR,
> > +               "Offset to next data block exceeds number of allocated data
> > blocks.\n");
> >
> >
> >      if (render->filled_mv_blocks_num == render->allocated_mv_blocks)
> 
> This is wrong. Revert it.
> 
> Asserts are used only to check for conditions that are impossible to occur
> if the logic of the program is correct.
> Failing  assert means somewhere (not necessary where the assert is)
> have gone wrong.

Then how do you interpret the following part of Michael's review?

  >    assert(render->filled_mv_blocks_num <= render->total_number_of_mv_blocks);
  >    assert(render->next_free_data_block_num <= render->total_number_of_data_blocks);

  5 points
  replace all assert() checks on all user settable fields by
  normal if() av_log() whatever else is needed checks

It seems clear to me that he wants those asserts replaced by checks like
the ones I implemented.

Diego