[FFmpeg-devel] [PATCH] BGR24 Huffyuv and drive-by bug fixes

[Michael Niedermayer]

On Thu, Oct 15, 2009 at 03:51:11PM -0400, Alexander Strange wrote:
> The Huffyuv decoder currently decodes all RGB files as RGB32 with the alpha 
> channel left uninitialized - there are rows of 0x80 (the default) and a few 
> of 0x00 (probably because of plane prediction).
> This has just recently caused some rendering problems in Perian, although 
> I'm not sure why it worked before.
>
> Attached patches decode 24-bit huffyuv as BGR24 to avoid this, and fix some 
> other problems I found along the way.
>
> 1- fixes a valgrind warning in get_vlc2, caused by the input padding in 
> bitstream_buffer being uninitialized.
> 2- fixes temp[0] and temp[1] being allocated when only temp[0] is used for 
> RGB.
> 3- adds 'const' to the DSP functions' src pointers (didn't run patcheck?) 
> and removes a meaningless 'inline'.
> 4- adds BGR24 decoding for 24-bit files. Decoding speed is about the same.
> It requires a bit of copied code; PIX_FMT_RGB32 is endian-dependent and 
> BGR24 isn't, so merging the two cases turned out very ugly.
> I used AV_WL32 to do a 24-bit write, since it's faster than AV_WL24 on most 
> platforms.
> 5- reindents.
>
> The last patch is meant to add correct alpha decoding for actual RGBA 
> files, but without a sample I can't test it.
>

>  huffyuv.c |    1 +
>  1 file changed, 1 insertion(+)
> b2c3eb9de71f07334e526a66c7020f95bb250fab  0001-Huffyuv-Fix-a-valgrind-warning-in-get_vlc2.patch
> From f1e20a1a2db0e19627a60923bbf157e7e0125c0f Mon Sep 17 00:00:00 2001
> From: Alexander Strange <astrange at ithinksw.com>
> Date: Wed, 14 Oct 2009 22:16:28 -0400
> Subject: [PATCH 1/6] Huffyuv: Fix a valgrind warning in get_vlc2().

ok

[...]
>  huffyuv.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 0b81e6982f54659034d214368885b7018113e112  0002-Huffyuv-Remove-unnecessary-allocation-in-alloc_temp.patch
> From 09cebd6409e9bb3f6b9ad47e99466c908aeac1c5 Mon Sep 17 00:00:00 2001
> From: Alexander Strange <astrange at ithinksw.com>
> Date: Wed, 14 Oct 2009 22:17:15 -0400
> Subject: [PATCH 2/6] Huffyuv: Remove unnecessary allocation in alloc_temp().
> 
> RGB only needs one temp array.
> ---
>  libavcodec/huffyuv.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/huffyuv.c b/libavcodec/huffyuv.c
> index 7fd03d8..b8e59ed 100644
> --- a/libavcodec/huffyuv.c
> +++ b/libavcodec/huffyuv.c
> @@ -406,9 +406,7 @@ static av_cold void alloc_temp(HYuvContext *s){
>              s->temp[i]= av_malloc(s->width + 16);
>          }
>      }else{
> -        for(i=0; i<2; i++){
> -            s->temp[i]= av_malloc(4*s->width + 16);
> -        }
> +            s->temp[0]= av_malloc(4*s->width + 16);

indent is wrong, except that ok


[...]

>  dsputil.c |    8 ++++----
>  dsputil.h |    8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 7234f90d352a3569fd62170e0850a3b1194ae11b  0003-Add-missing-const-for-src-pointers-in-Huffyuv-dsp-fu.patch
> From 0cba382264e9717c3461086b1090c0558b440e35 Mon Sep 17 00:00:00 2001
> From: Alexander Strange <astrange at ithinksw.com>
> Date: Thu, 15 Oct 2009 00:26:56 -0400
> Subject: [PATCH 3/6] Add missing const for src pointers in Huffyuv dsp functions.
> 
> Also remove a useless 'inline'.

ok though maybe that should be 2 commits, i see no relation between adding
const and removing inline


[...]

>  dsputil.c |   23 +++++++++++++++++++++++
>  dsputil.h |    1 +
>  huffyuv.c |   62 +++++++++++++++++++++++++++++++++++++++-----------------------
>  3 files changed, 63 insertions(+), 23 deletions(-)
> b5040e16dc7f1c8edd5b85a99a11f4e32f3ee521  0004-Huffyuv-Decode-RGB24-as-PIX_FMT_BGR24.patch
> From 268f8db45761bbf65140c88b6c82d0c2db57225f Mon Sep 17 00:00:00 2001
> From: Alexander Strange <astrange at ithinksw.com>
> Date: Wed, 14 Oct 2009 22:15:28 -0400
> Subject: [PATCH 4/6] Huffyuv: Decode RGB24 as PIX_FMT_BGR24.
> 
> Currently it decodes as RGB32 with junk in the alpha channel.
> ---
>  libavcodec/dsputil.c |   23 ++++++++++++++++++
>  libavcodec/dsputil.h |    1 +
>  libavcodec/huffyuv.c |   62 +++++++++++++++++++++++++++++++------------------
>  3 files changed, 63 insertions(+), 23 deletions(-)

it might be as fast to handle 3 byte groups in C but its not clear how SIMD
would behave with that


[...]

>  dsputil.c |   11 +++++++++--
>  dsputil.h |    2 +-
>  huffyuv.c |   12 +++++++-----
>  3 files changed, 17 insertions(+), 8 deletions(-)
> af61a77328bc033b2a9f9a74535322e51504c1d4  0006-Huffyuv-Decode-the-alpha-channel-in-RGB32-files.patch
> From b492be90bf4a0f512daec72b20e53aafbbde7356 Mon Sep 17 00:00:00 2001
> From: Alexander Strange <astrange at ithinksw.com>
> Date: Wed, 14 Oct 2009 22:27:47 -0400
> Subject: [PATCH 6/6] Huffyuv: Decode the alpha channel in RGB32 files.
> 

> Untested.

untested is not good ...

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

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091015/56e8f556/attachment.pgp>