[Ffmpeg-devel] [PATCH] THP PCM decoder (GSoC Qualification)
Tue Apr 3 20:59:31 CEST 2007
Michael Niedermayer <michaelni at gmx.at> writes:
> 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
I tried about anything. This is the first time I am working with
audio, so I perhaps made a silly mistake.
What I tried was interleaving the data send to the output buffer. The
PCM data in the THP files is stored in packets. First x samples for
the first channel are stored, after that x samples for the second
When I decode the other channel it seems to work as well. So I assume
the problem is in how I fill the output buffer and how I tell ffmpeg
the format (stereo, interleaved samples) of this data. How does one
in general deal with stereo from a decoder?
>> 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 ?
Baptiste said "use CODEC and dummy return -1 to stay in conformance
with others (see IMA_QT)". I assumed it also applied to allcodecs.c.
Should I change this to REGISTER_DECODER?
>> 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 ...
Same as above. Should I remove this?
>> + 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
More information about the ffmpeg-devel