[Ffmpeg-devel] [PATCH] THP PCM decoder (GSoC Qualification)

[Michael Niedermayer]

Hi

On Tue, Apr 03, 2007 at 03:21:56PM +0200, Marco Gerards wrote:
[...]
> In this new patch there are still two issues.  For some reason the
> quality of the sound was not that good.  It wasn't as bad as someone
> reported, for example he said that there were issues with mono sound.
> I can not reproduce this.
> 
> The problem is in stereo sound.  I have disabled stereo in this patch
> and the sound is just fine now.  To be honest, I am not sure what the
> problem is.  I have tried all kinds of things without much success.
> Perhaps it is better to commit this first.  After that I can fix this
> with another patch.

hmm did you properly interleave right and left channel samples? that is
does it work if you decode just the other channel?
could it be that the encoding is R+L,R-L ? instead of R,L


> 
> Another thing is the seek.  Is this a big problem?  I am only seeking
> forwards, so I made the assumption that ffmpeg just skips the data if
> it cannot seek.  

yes it should

> Or is this something I should look at?  It's not
> documented how many padding there is.
> 
> --
> Marco
> 
> 
> Index: libavcodec/allcodecs.c
> ===================================================================
> --- libavcodec/allcodecs.c	(revision 8605)
> +++ libavcodec/allcodecs.c	(working copy)
> @@ -242,6 +242,7 @@
>      REGISTER_ENCDEC (ADPCM_SBPRO_3, adpcm_sbpro_3);
>      REGISTER_ENCDEC (ADPCM_SBPRO_4, adpcm_sbpro_4);
>      REGISTER_ENCDEC (ADPCM_SWF, adpcm_swf);
> +    REGISTER_ENCDEC (ADPCM_THP, adpcm_thp);

ENCDEC ?


>      REGISTER_ENCDEC (ADPCM_XA, adpcm_xa);
>      REGISTER_ENCDEC (ADPCM_YAMAHA, adpcm_yamaha);
>  
> Index: libavcodec/Makefile
> ===================================================================
> --- libavcodec/Makefile	(revision 8605)
> +++ libavcodec/Makefile	(working copy)
> @@ -246,6 +246,8 @@
>  OBJS-$(CONFIG_ADPCM_SBPRO_4_ENCODER)   += adpcm.o
>  OBJS-$(CONFIG_ADPCM_SWF_DECODER)       += adpcm.o
>  OBJS-$(CONFIG_ADPCM_SWF_ENCODER)       += adpcm.o
> +OBJS-$(CONFIG_ADPCM_THP_DECODER)       += adpcm.o
> +OBJS-$(CONFIG_ADPCM_THP_ENCODER)       += adpcm.o
>  OBJS-$(CONFIG_ADPCM_XA_DECODER)        += adpcm.o

unless you write an ADPCM_THP encoder there shouldnt be
a CONFIG_ADPCM_THP_ENCODER ...


[...]
> +        init_get_bits(&gb, src, buf_size * 8);
> +        src += buf_size;
> +
> +                    get_bits(&gb, 32); /* Channel size */
> +        samplecnt = get_bits(&gb, 32);

get_bits is not guranteed to be able to read 32 bit, use get_bits_long()
or better yet, use bytestream_get_be32() and similar they are faster and
simpler for bytealigned stuff


[...]

> +       thp->audiosize = get_be32(pb); /* Audio size.  */

comment is redundant

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070403/b90aa0f6/attachment.pgp>