[FFmpeg-devel] [PATCH] mpeg2: fix block_last_index when mismatch control modifies last coeff

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Tue, Jun 22, 2010 at 09:46:35PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> >> +static inline void idct_dc_add(uint8_t *dst, int line_size, int dc)
>> >> +{
>> >> +    int x, y;
>> >> +    uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;
>> >
>> >> +    dc = (16383 * dc + 1024) >> 11;
>> >> +    dc = (16383 * (dc + 32)) >> 20;
>> >
>> > dc= (dc + (i<0) + 3)>>3;
>> 
>> i?
>
> i=dc

OK.  This could compile to a branch with some back luck, ...

>> > or
>> > dc= (dc*2047 + 8192)>>14;

... while this shouldn't.

>> >> +    for (y = 0; y < 8; y++, dst += line_size) {
>> >> +        for (x = 0; x < 8; x++) {
>> >> +            dst[x] = cm[dst[x] + dc];
>> >> +        }
>> >> +    }
>> >
>> > cm += dc;
>> > can be done outside the loop
>> 
>> I noticed today this could be done in quite a few places throughout
>> existing code.
>
> please fix them

I will.

>> >> Index: libavcodec/h264.h
>> >> ===================================================================
>> >> --- libavcodec/h264.h	(revision 23459)
>> >> +++ libavcodec/h264.h	(working copy)
>> >> @@ -735,22 +735,6 @@
>> >>   1+5*8, 2+5*8,
>> >>  };
>> >>  
>> >> -static av_always_inline uint32_t pack16to32(int a, int b){
>> >> -#if HAVE_BIGENDIAN
>> >> -   return (b&0xFFFF) + (a<<16);
>> >> -#else
>> >> -   return (a&0xFFFF) + (b<<16);
>> >> -#endif
>> >> -}
>> >> -
>> >> -static av_always_inline uint16_t pack8to16(int a, int b){
>> >> -#if HAVE_BIGENDIAN
>> >> -   return (b&0xFF) + (a<<8);
>> >> -#else
>> >> -   return (a&0xFF) + (b<<8);
>> >> -#endif
>> >> -}
>> >> -
>> >
>> > moving these or anything else to a common place is ok of course
>> 
>> I suggest alongside the 8x4 Ronald added today.
>
> just keep track which of these work with negative numbers and which not

Maybe we should have one of each to avoid needless masking:

PACK_2S8
PACK_2U8
PACK_4S8
PACK_4U8
PACK_2S16
PACK_2U16

-- 
M?ns Rullg?rd
mans at mansr.com