[Ffmpeg-devel] [PATCH] THP PCM decoder (GSoC Qualification)
Michael Niedermayer
michaelni
Thu Apr 5 00:26:44 CEST 2007
Hi
On Wed, Apr 04, 2007 at 10:30:29PM +0200, Marco Gerards wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
>
> >> >> 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
> >> >> channel.
> >> >>
> >> >> 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?
> >> >
> >> > stereo in ffmpeg is always
> >> > channel0-sample0, channel1-sample0, channel0-sample1, channel1-sample1, channel0-sample2, channel1-sample2, ...
> >>
> >> Well, I have this now and it works... kinda. When I am playing either
> >> one of the two channels as mono sound, it works. When I am using both
> >> channels, there is some kind of noise/cracks. The noise is not there
> >> all the time and it is not very loud, but I am mentioning this because
> >> there must still be something that I do wrong...
> >
> > hmm, is AVCodecContext.channels set correctly
> > does it also sound bad if you dump it to a file or let ffmpeg transcode it
> > to pcm wav and then play that ... ?
>
> The problem was indeed a silly bug in my code. I increased samples
> once too often, so the output buffer was reported to be one sample
> bigger than it actually was. I fixed this and all other mistakes you
> found.
>
> If you do not see any problems with the way I fixed this bug, I think
> this patch is ready to be committed. :-)
almost, i just found a few more minor things :)
[...]
> + long table[16][2];
why not int? the values stored in it seem to be limited to 16bit ...
[...]
> + for (ch = 0; ch <= st; ch++) {
> + samples = (unsigned short *) data + ch;
> +
> + /* Read in every sample for this channel. */
> + for (i = 0; i < samplecnt / 14; i++) {
> + uint8_t index = get_bits (&gb, 4) & 7;
> + unsigned int exp = 1 << get_bits (&gb, 4);
storing "log2"(exp) instead of exp would avoid a multiply
> + signed long factor1 = table[index * 2][ch];
isnt int enough?
> + signed long factor2 = table[index * 2 + 1][ch];
> +
> + /* Decode 14 samples. */
> + for (n = 0; n < 14; n++) {
> + int sampledat = get_sbits (&gb, 4);
> +
> + *samples = ((prev1[ch]*factor1
trailing whitespace
[...]
> + else {
> + ret = av_get_packet(pb, pkt, thp->audiosize);
> + if (ret != thp->audiosize) {
> + av_free_packet(pkt);
> + return AVERROR_IO;
> + }
>
> + pkt->stream_index = thp->audio_stream_index;
> + thp->audiosize = 0;
> + thp->frame++;
> + }
indention is not 4 spaces and it seems the existing code in thp.c is also
not consistently indented, somehow i must have missed that ...
this should be fixed
and of course the existing code reindention should be seperate from functional
changes
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/20070405/39615956/attachment.pgp>
More information about the ffmpeg-devel
mailing list