[FFmpeg-devel] [PATCH v3] lavc/rawdec: Use AV_PIX_FMT_PAL8 for 1-bit raw AVI video

[Michael Niedermayer]

On Mon, Jan 25, 2016 at 01:30:01AM +0100, Mats Peterson wrote:
> On 01/25/2016 01:26 AM, Michael Niedermayer wrote:
> >On Wed, Jan 20, 2016 at 12:41:22PM +0100, Mats Peterson wrote:
> >>I don't know about this one, since it adds some calculations inside
> >>the loop, but it limits the line alignment to 16 bytes instead of 32
> >>bytes as before. Less overhead.
> >>
> >>Mats
> >>
> >>--
> >>Mats Peterson
> >>http://matsp888.no-ip.org/~mats/
> >
> >>  raw.c    |    2 +-
> >>  rawdec.c |   38 ++++++++++++++++++++++++--------------
> >>  2 files changed, 25 insertions(+), 15 deletions(-)
> >>3c94aff2647b8a1b0e9c4c147b552c85255253ff  0001-v3.patch
> >> From 8d15b348fc84d46f7e764a2e9822aacc80e74407 Mon Sep 17 00:00:00 2001
> >>From: Mats Peterson <matsp888 at yahoo.com>
> >>Date: Wed, 20 Jan 2016 12:37:40 +0100
> >>Subject: [PATCH v3] lavc/rawdec: Use AV_PIX_FMT_PAL8 for 1-bit raw AVI video
> >>
> >>The stuff about 1-bit video not necessarily being black & white in
> >>QuickTime goes for AVI as well. Being 1 bit per pixel only means that
> >>the data is bi-level. The two colors can be any color. Since many
> >>1 bpp AVI files don't have a palette following the BITMAPINFOHEADER,
> >>I'm setting a "default" black & white palette in raw_init().
> >
> >Does the official player from microsoft play such crafted avi files
> >with the palette colors or black and white ?
> >
> >
> >[...]
> >>@@ -94,8 +94,11 @@ static av_cold int raw_init_decoder(AVCodecContext *avctx)
> >>              return AVERROR(ENOMEM);
> >>          if (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)
> >>              avpriv_set_systematic_pal2((uint32_t*)context->palette->data, avctx->pix_fmt);
> >>-        else
> >>+        else {
> >>              memset(context->palette->data, 0, AVPALETTE_SIZE);
> >>+            if (avctx->bits_per_coded_sample == 1)
> >>+                memset(context->palette->data, 0xff, 4);
> >>+        }
> >
> >hunk split out and applied
> >
> >
> >[...]
> >
> >>@@ -222,18 +225,25 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
> >>              }
> >>              linesize_align = 16;
> >>          } else {
> >>+            int j, row_pix = 0;
> >>              av_assert0(avctx->bits_per_coded_sample == 1);
> >>-            for (i = 0; 8 * i + 7 < buf_size && i<avpkt->size; i++) {
> >>-                dst[8 * i + 0] = buf[i] >> 7 & 1;
> >>-                dst[8 * i + 1] = buf[i] >> 6 & 1;
> >>-                dst[8 * i + 2] = buf[i] >> 5 & 1;
> >>-                dst[8 * i + 3] = buf[i] >> 4 & 1;
> >>-                dst[8 * i + 4] = buf[i] >> 3 & 1;
> >>-                dst[8 * i + 5] = buf[i] >> 2 & 1;
> >>-                dst[8 * i + 6] = buf[i] >> 1 & 1;
> >>-                dst[8 * i + 7] = buf[i]      & 1;
> >>+            for (i = 0, j = 0; 8 * j + 7 < buf_size && i<avpkt->size; i++, j++) {
> >>+                dst[8 * j + 0] = buf[i] >> 7 & 1;
> >>+                dst[8 * j + 1] = buf[i] >> 6 & 1;
> >>+                dst[8 * j + 2] = buf[i] >> 5 & 1;
> >>+                dst[8 * j + 3] = buf[i] >> 4 & 1;
> >>+                dst[8 * j + 4] = buf[i] >> 3 & 1;
> >>+                dst[8 * j + 5] = buf[i] >> 2 & 1;
> >>+                dst[8 * j + 6] = buf[i] >> 1 & 1;
> >>+                dst[8 * j + 7] = buf[i]      & 1;
> >>+                row_pix += 8;
> >>+                if (row_pix >= avctx->width) {
> >>+                    i += 4 - (i % 4) - 1;
> >>+                    j += 2 - (j % 2) - 1;
> >>+                    row_pix = 0;
> >>+                }
> >>              }
> >>-            linesize_align = 32;
> >>+            linesize_align = 16;
> >>          }
> >
> >hunk split out and applied
> >
> >[...]
> >
> >
> >
> >_______________________________________________
> >ffmpeg-devel mailing list
> >ffmpeg-devel at ffmpeg.org
> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> Well, this one is still incorrect, since it doesn't cater for
> different strides.

can you post a patchset with split changes ?
that is one bugfix -> one patch, one feature -> one patch

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160125/613c3766/attachment.sig>