[FFmpeg-cvslog] r14692 - in trunk: libavcodec/pcm.c tests/regression.sh

[Michael Niedermayer]

On Tue, Aug 12, 2008 at 01:52:24PM -0300, Ramiro Polla wrote:
> pross at xvid.org wrote:
>> On Tue, Aug 12, 2008 at 09:58:32AM -0300, Ramiro Polla wrote:
>>> Ramiro Polla wrote:
>>>> Hi,
>>>>
>>>>> the number of decoded U8 samples is
>>>>> calculated using some lazy logic (number of short ints / 2). 
>>>>> See patch.
>>>> Thanks. I saw this problem when implementing 24-bit support for the MLP 
>>>> encoder.
>>> Oh, this change only dealt with 8-bit samples. I hadn't seen that (I 
>>> haven't actually tested it =). But I think the same problem arises with 
>>> higher bitrates.
>>> Trying ./ffmpeg_g -i input.wav -sample_fmt s32 output.mlp
>>> (assuming mlp accepts SAMPLE_FMT_S32) only gives half the samples. So if 
>>> avctx->frame_size is 40, I only get 20 samples...
>> This looks to be a "1sample = 2bytes" legacy assumption within ffmpeg.
>> Try the enclosed patch.
>
> I had to change another 2 to av_get_bits_per_sample_format()/8. I attached 
> the 2 patches I had to apply to get 24-bit working on MLP. This way it 
> works with:
> ./ffmpeg_g -i file16bit.wav -sample_fmt s24 file24bit.mlp
>
> But with this I also had to read the 24-bit samples with (int32_t) 
> ((bytestream_get_le24() >> 8) << 8). I guess what Michael wants is to be 
> able to use (int32_t) bytestream_get_le32() directly. I'll leave the mlp 
> code with bytestream_get_le32() for now...

[...]


> Index: libavcodec/audioconvert.c
> ===================================================================
> --- libavcodec/audioconvert.c	(revision 14695)
> +++ libavcodec/audioconvert.c	(working copy)
> @@ -129,8 +129,10 @@
>          else CONV(SAMPLE_FMT_FLT, float  , SAMPLE_FMT_U8 , (*(uint8_t*)pi - 0x80)*(1.0 / (1<<7)))
>          else CONV(SAMPLE_FMT_U8 , uint8_t, SAMPLE_FMT_S16, (*(int16_t*)pi>>8) + 0x80)
>          else CONV(SAMPLE_FMT_S16, int16_t, SAMPLE_FMT_S16,  *(int16_t*)pi)
> +        else CONV(SAMPLE_FMT_S24, int32_t, SAMPLE_FMT_S16,  *(int16_t*)pi<< 8)
>          else CONV(SAMPLE_FMT_S32, int32_t, SAMPLE_FMT_S16,  *(int16_t*)pi<<16)
>          else CONV(SAMPLE_FMT_FLT, float  , SAMPLE_FMT_S16,  *(int16_t*)pi*(1.0 / (1<<15)))
> +        else CONV(SAMPLE_FMT_S16, int16_t, SAMPLE_FMT_S24,  *(int32_t*)pi>> 8)
>          else CONV(SAMPLE_FMT_U8 , uint8_t, SAMPLE_FMT_S32, (*(int32_t*)pi>>24) + 0x80)
>          else CONV(SAMPLE_FMT_S16, int16_t, SAMPLE_FMT_S32,  *(int32_t*)pi>>16)
>          else CONV(SAMPLE_FMT_S32, int32_t, SAMPLE_FMT_S32,  *(int32_t*)pi)

if we do not drop SAMPLE_FMT_S24 entirely then it should be identical to
SAMPLE_FMT_S32, that is the 8lsb should be 0.
Thats makes handling much simpler as a if(x==S24)x = S32; can often
be used.

and iam not against the other patch if it has been tested

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

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- 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-cvslog/attachments/20080812/4665987d/attachment.pgp>