[FFmpeg-devel] [PATCH] avcodec/s3tc: fix decoding when dimensions are not a multiple of 4

[Tom Butterworth]

I took variables bx and by to be shorthand for block x and block y, so I
renamed them because they now refer to pixel and not block positions -
would you rather they remain bx, by?

Examples at

http://files.kriss.cx/387x249DXT1.txd
http://files.kriss.cx/387x249DXT3.txd



On Thu, May 7, 2015 at 4:30 PM Michael Niedermayer <michaelni at gmx.at> wrote:

> On Thu, May 07, 2015 at 04:17:10PM +0100, Tom Butterworth wrote:
> > Image dimensions were rounded down causing rows to wrap early if
> dimensions were not a multiple of 4.
> > ---
> >  libavcodec/s3tc.c | 23 +++++++++++++----------
> >  1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/libavcodec/s3tc.c b/libavcodec/s3tc.c
> > index 4743d78..9886b1d 100644
> > --- a/libavcodec/s3tc.c
> > +++ b/libavcodec/s3tc.c
> > @@ -26,6 +26,7 @@
> >  #include "s3tc.h"
> >
> >  static inline void dxt1_decode_pixels(GetByteContext *gb, uint32_t *d,
> > +                                      unsigned int w, unsigned int h,
> >                                        unsigned int qstride, unsigned
> int flag,
> >                                        uint64_t alpha) {
> >      unsigned int x, y, c0, c1, a = (!flag * 255u) << 24;
> > @@ -62,14 +63,16 @@ static inline void dxt1_decode_pixels(GetByteContext
> *gb, uint32_t *d,
> >      colors[2] = rb2 + g2 + a;
> >
> >      pixels = bytestream2_get_le32(gb);
> > -    for (y=0; y<4; y++) {
> > -        for (x=0; x<4; x++) {
> > +    for (y=0; y<h; y++) {
> > +        for (x=0; x<w; x++) {
> >              a        = (alpha & 0x0f) << 28;
> >              a       += a >> 4;
> >              d[x]     = a + colors[pixels&3];
> >              pixels >>= 2;
> >              alpha  >>= 4;
> >          }
> > +        for (; x<4; x++)
> > +            pixels >>= 2;
> >          d += qstride;
> >      }
> >  }
>
> > @@ -77,21 +80,21 @@ static inline void dxt1_decode_pixels(GetByteContext
> *gb, uint32_t *d,
> >  void ff_decode_dxt1(GetByteContext *gb, uint8_t *dst,
> >                      const unsigned int w, const unsigned int h,
> >                      const unsigned int stride) {
> > -    unsigned int bx, by, qstride = stride/4;
> > +    unsigned int x, y, qstride = stride/4;
> >      uint32_t *d = (uint32_t *) dst;
> >
> > -    for (by=0; by < h/4; by++, d += stride-w)
> > -        for (bx = 0; bx < w / 4; bx++, d += 4)
> > -            dxt1_decode_pixels(gb, d, qstride, 0, 0LL);
> > +    for (y=0; y < h; y += 4, d += stride-w)
> > +        for (x = 0; x < w; d += FFMIN(4, w-x), x += 4)
> > +            dxt1_decode_pixels(gb, d, FFMIN(4, w-x), FFMIN(4, h-y),
> qstride, 0, 0LL);
> >  }
>
> please dont rename variables in patches which contain functional
> changes
>
> also, can you share the sample that this patch fixes decoding for ?
>
> Thanks
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Good people do not need laws to tell them to act responsibly, while bad
> people will find a way around the laws. -- Plato
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>