[FFmpeg-devel] [PATCH] Bink Video decoder for FFmpeg 0.6
Måns Rullgård
mans
Tue Feb 16 21:00:34 CET 2010
Jason Garrett-Glaser <darkshikari at gmail.com> writes:
> I'll try my hand at some patch review.
>
> + do {
> + if (!get_bits1(gb)) {
> + *dst++ = *src++;
> + size--;
> + } else {
> + *dst++ = *src2++;
> + size2--;
> + }
> + } while (size && size2);
>
> Maybe dst++ should be moved out of the if? I don't trust gcc to
> optimize away the duplicated increment that would otherwise be in each
> branch.
OTOH, keeping it as is allows post-increment addressing modes. I
don't trust gcc to optimise the increment _into_ the branches either.
> + while (size--)
> + *dst++ = *src++;
> + while (size2--)
> + *dst++ = *src2++;
>
> This looks like a memcpy to me.
This one is defined for overlapping blocks. Don't know if it matter here.
> + if (!bits) {
> + t = get_bits1(gb) ? -1 : 1;
> + } else {
> + t = get_bits(gb, bits) | mask;
> + if (get_bits1(gb))
> + t = -t;
> + }
>
> Is get_bits valid with bits=0? If so, this can be simplified.
get_bits(0) returns an undefined value, but is guaranteed not to
advance the bitstream.
--
M?ns Rullg?rd
mans at mansr.com
More information about the ffmpeg-devel
mailing list