[FFmpeg-devel] [PATCH] bink: perform scan order permutation of quantization matrices in read_dct_coeffs()

[Ronald S. Bultje]

Hi,

On Sat, Feb 19, 2011 at 9:30 PM, Peter Ross <pross at xvid.org> wrote:
> On Sat, Feb 19, 2011 at 07:29:06PM -0500, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Sun, Feb 13, 2011 at 4:29 AM, Peter Ross <pross at xvid.org> wrote:
>> > On Sun, Feb 13, 2011 at 07:41:32AM +0100, Kostya wrote:
>> >> On Sun, Feb 13, 2011 at 02:32:58PM +1100, Peter Ross wrote:
>> >> > This fixes visual glitches in Bink version 'b' files, as the quantization tables
>> >> > were not being permutated.
>> >> > ---
>> >> > ?libavcodec/bink.c ? ? | ? ?4 +-
>> >> > ?libavcodec/binkdata.h | ?618 ++++++++++++++++++++++++-------------------------
>> >> > ?2 files changed, 303 insertions(+), 319 deletions(-)
>> >> >
>> >> > diff --git a/libavcodec/bink.c b/libavcodec/bink.c
>> >> > index 928bf53..5ebf98c 100644
>> >> > --- a/libavcodec/bink.c
>> >> > +++ b/libavcodec/bink.c
>> >> > @@ -678,8 +678,8 @@ static int read_dct_coeffs(GetBitContext *gb, DCTELEM block[64], const uint8_t *
>> >> >
>> >> > ? ? ?block[0] = (block[0] * quant[0]) >> 11;
>> >> > ? ? ?for (i = 0; i < coef_count; i++) {
>> >> > - ? ? ? ?int idx = coef_idx[i];
>> >> > - ? ? ? ?block[scan[idx]] = (block[scan[idx]] * quant[idx]) >> 11;
>> >> > + ? ? ? ?int idx = scan[coef_idx[i]];
>> >> > + ? ? ? ?block[idx] = (block[idx] * quant[idx]) >> 11;
>> >> > ? ? ?}
>> >>
>> >> 1) permutation depends on DCT type selected. Maybe in future Jason will
>> >> fullfill his promise and write optimised DCT for Bink and it will require
>> >> different permutation. Then decoder will be broken.
>> >
>> > True. My oops, the critical lines should therefore be:
>> >
>> > ? ? ? ?int idx = coef_idx[i];
>> > ? ? ? ?block[scan[idx]] = (block[scan[idx]] * quant[bink_scan[idx]]) >> 11;
>> >
>> >> 2) you generate bink-b tables anyway so you can permutate them during creation
>> >> unlike static data we have for latter Bink versions.
>> >
>> > Ok, for now. We'll need to revisit this when the Bink encoder is committed.
>>
>> Will you send a new patch for this?
>
> Already done! I send a alternative patch to this thread a week ago, Koysta quickly
> approved, and Mans commited it.

Oh shit I missed it, apologies.

Ronald