[FFmpeg-devel] [PATCH] cllc: simplify/fix swapped data buffer allocation.

[Reimar Döffinger]

On Sun, Aug 26, 2012 at 01:14:08PM -0400, Derek Buitenhuis wrote:
> On 26/08/2012 7:11 AM, Reimar Döffinger wrote:
> > Using the malloc variant avoids pointless memcpy on size
> > increase and simplifies handling allocation failure.
> > Also change code to ensure that allocation, bswap and bitstream
> > reader all use the same size, even when the packet size is odd
> > for example.
> > 
> > Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> > ---
> >  libavcodec/cllc.c |   30 +++++++++++++-----------------
> >  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> I'm for this patch, but I would like to know if there is a
> measurable difference.

I couldn't see anything over the measurement error for the fate sample,
but I did not test particularly carefully (only used "time", not the
CPU timers).
The minimum user time stayed 0.104, though the most frequent value
changed from 0.112 to 0.108 - but I don't think it's significant
and I don't feel like spending much time on it.

> > +    data_size = (avpkt->size - info_offset) & ~1;
> 
> Won't this technically not meet the requirement of init_get_bits:
> 
> "bitstream buffer, must be FF_INPUT_BUFFER_PADDING_SIZE bytes larger
> than the actual read bits because some optimized bitstream readers
> read 32 or 64 bit at once and could read over the end."
> 
> This could end up only FF_INPUT_BUFFER_PADDING_SIZE - 1 bytes larger.

I use data_size * 8 for init_get_bits, so I do not see that?

> > +    /* Make sure our bswap16'd buffer is big enough */
> > +    av_fast_padded_malloc(&ctx->swapped_buf,
> > +                          &ctx->swapped_buf_size, data_size);
> > +    if (!ctx->swapped_buf) {
> > +        av_log(avctx, AV_LOG_ERROR, "Could not realloc swapped buffer.\n");
> 
> Perhaps this should be changed? It's not technically a realloc.

Yes, changed "realloc" to "allocate" locally.