[FFmpeg-devel] [PATCH v9] lavc/rawdec: Remove monowhite switching code for 1 bpp AVI without a palette

[Michael Niedermayer]

On Fri, Feb 12, 2016 at 11:26:02AM +0100, Mats Peterson wrote:
> Now handles non-standard 8 bpp raw AVI files created with "-vcodec
> rawvideo" that contain the palette at the end of each frame, like
> nut.
> 
> 8 bpp raw QuickTime files created with "-vcodec rawvideo" won't work
> quite right, because they contain a palette both in the video sample
> description and at the end of each frame, which currently causes the
> stride to be calculated incorrectly. They are hugely non-standard
> anyway.
> 
> Original description follows:
> 
> This patch removes the monowhite switching code for 1 bpp raw AVI
> without a palette. I don't see any reason to keep it, since it's
> semantically incorrect to use monowhite for palettized data in my
> book, it adds unnecessary noise to the code, and it's inconsistent
> with the rest of the code in FFmpeg.
> 
> For black & white AVI or QuickTime files, in order to produce a
> monochrome nut file, force the pixel format with "-pix_fmt monow" or
> "-pix_fmt monob".
> 
> Another way is to use "ffmpeg -i 1bpp.avi -vcodec copy -vtag B1W0
> 1bpp.nut". The "-vtag" option is currently needed, otherwise FFmpeg
> will use a RGB[15] codec tag for some reason.
> 
> I have also updated the 1 bpp FATE test files (once again).
> 
> Mats
> 
> -- 
> Mats Peterson
> http://matsp888.no-ip.org/~mats/

>  libavcodec/rawdec.c               |   68 ++++++++++++++++----------------------
>  tests/ref/vsynth/vsynth1-bpp1     |    4 +-
>  tests/ref/vsynth/vsynth2-bpp1     |    4 +-
>  tests/ref/vsynth/vsynth3-bpp1     |    4 +-
>  tests/ref/vsynth/vsynth_lena-bpp1 |    4 +-
>  5 files changed, 37 insertions(+), 47 deletions(-)
> 2023febf5ae59cd14dd658c30cebeab2e10519fe  0001-lavc-rawdec-Remove-monowhite-switching-code-for-1-bp.patch
> From 1600eeb442b1ebfd56a7f8882b93db3f2076b006 Mon Sep 17 00:00:00 2001
> From: Mats Peterson <matsp888 at yahoo.com>
> Date: Fri, 12 Feb 2016 11:24:30 +0100
> Subject: [PATCH v9] lavc/rawdec: Remove monowhite switching code for 1 bpp AVI without a palette
> 
> ---
>  libavcodec/rawdec.c               |   68 ++++++++++++++++---------------------
>  tests/ref/vsynth/vsynth1-bpp1     |    4 +--
>  tests/ref/vsynth/vsynth2-bpp1     |    4 +--
>  tests/ref/vsynth/vsynth3-bpp1     |    4 +--
>  tests/ref/vsynth/vsynth_lena-bpp1 |    4 +--
>  5 files changed, 37 insertions(+), 47 deletions(-)
> 
> diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
> index b7ce2b6..cadeaff 100644
> --- a/libavcodec/rawdec.c
> +++ b/libavcodec/rawdec.c
> @@ -46,6 +46,7 @@ typedef struct RawVideoContext {
>      int is_pal8;
>      int is_nut_mono;
>      int is_nut_pal8;
> +    int has_pkt_pal;
>      int is_yuv2;
>      int is_lt_16bpp; // 16bpp pixfmt and bits_per_coded_sample < 16
>      int tff;
> @@ -100,7 +101,7 @@ static av_cold int raw_init_decoder(AVCodecContext *avctx)
>              avpriv_set_systematic_pal2((uint32_t*)context->palette->data, avctx->pix_fmt);
>          else {
>              memset(context->palette->data, 0, AVPALETTE_SIZE);
> -            if (avctx->bits_per_coded_sample <= 1)
> +            if (avctx->bits_per_coded_sample == 1)
>                  memset(context->palette->data, 0xff, 4);
>          }
>      }
> @@ -121,17 +122,13 @@ static av_cold int raw_init_decoder(AVCodecContext *avctx)

>      if (avctx->codec_tag == MKTAG('B','1','W','0') ||
>          avctx->codec_tag == MKTAG('B','0','W','1'))
>          context->is_nut_mono = 1;
> -    else if (avctx->codec_tag == MKTAG('P','A','L','8'))
> +    else if (avctx->codec_tag == MKTAG('P','A','L',8))
>          context->is_nut_pal8 = 1;
>  

please split the patch so that each patch is selfcontained and
does not contain independant changes or bugfixes.
see "git rebase -i" and 'git add -p' and "git commit --amend" for
tools to conveniently seperate changes into commits

It is important for git history, future bisect/debug and even
yourself understanding changes in the future to cleanly split
commits.


>      if (avctx->codec_tag == AV_RL32("yuv2") &&
>          avctx->pix_fmt   == AV_PIX_FMT_YUYV422)
>          context->is_yuv2 = 1;
>  
> -    /* Temporary solution until PAL8 is implemented in nut */
> -    if (context->is_pal8 && avctx->bits_per_coded_sample == 1)
> -        avctx->pix_fmt = AV_PIX_FMT_NONE;
> -
>      return 0;
>  }
>  
> @@ -192,33 +189,21 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
>          return AVERROR_INVALIDDATA;
>      }
>  
> -    if (context->is_nut_mono)
> -        stride = avctx->width / 8 + (avctx->width & 7 ? 1 : 0);
> -    else if (context->is_nut_pal8)
> -        stride = avctx->width;
> -    else
> -        stride = avpkt->size / avctx->height;
> +    if ((avctx->bits_per_coded_sample == 8 || context->is_nut_pal8) &&
> +        avctx->frame_number == 0 &&
> +        !av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, NULL))
> +        context->has_pkt_pal = 1;
> +
> +    stride = (avpkt->size - (context->has_pkt_pal ? AVPALETTE_SIZE : 0)) / avctx->height;
>  
> -    if (stride == 0 || avpkt->size < stride * avctx->height) {

> +    av_log(avctx, AV_LOG_DEBUG, "PACKET SIZE: %d\n", avpkt->size);
> +    av_log(avctx, AV_LOG_DEBUG, "STRIDE: %d\n", stride);

this looks like some private debug code, which doesnt belong in this
patch

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- 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/20160212/f51fe22c/attachment.sig>