[FFmpeg-devel] [PATCH] GSM-MS decoder and encoder

Michael Niedermayer michaelni
Mon Apr 28 18:56:32 CEST 2008


On Mon, Apr 28, 2008 at 05:16:21PM +0200, Michel Bardiaux wrote:
> Michael Niedermayer wrote:
[...]

>>> its the latter that should fix things if the file is obviously corrupted 
>>> (which this one is!). 
>> I see nothing corrupt on the file
>>> That is, at the end of get_wav_header (proper patch when there is 
>>> agreement of the ways and means). Theoretically there is no maintainer 
>>> for riff.c but since Michael is maintainer for the avi muxdemux I guess 
>>> he is for riff.c too (should I correct MAINTAINERS?)
>> Yes i mainain riff.c, feel free to add me to the list if you like ...
>>> And whether the demuxer or the codec changes the sample rate, a warning 
>>> should be issued. OK?
>> Print as many warnings as you like :)
>> but please dont reject streams at random, 
>
> It is *NOT* at random. The spec is very clear: the sample rate for GSM is 
> 8000. Any other value in the RIFF headers is simply wrong and hints that 
> the encoder has screwed up.
>
>> patch below fixes this file and
>> i suspect others as well, i will apply it in 24h unless you object.
>
> The change was submitted for review and you didn't object... But if you 
> insist on a quick-and-dirty fix for all those malformed files, patch 
> attached for your consideration. But consider the consequences:

Your patch does not fix the problem just try to play them
with ffplay. My patch works, yours does not.
That file does have a sample rate of 44100 not 8000. The easiest solution
is simply to remove the code messing with the sample rate.
The alternative would be to add a sample_rate field to AVStream so both
demuxer and decoder can provide their sample rates to the user app. But
this seems really bad design as we know which rate is wrong and which is
not. And 2 sample rate fields will just force the user app to duplicate
the logic, the situation would be different if we didnt knew what was
correct but heres its clear, if the demuser says X its X if the demuxer
doesnt say anything its 8000.

Most sane would be
if(!avctx->sample_rate)
    avctx->sample_rate= 8000;

IMO

[...]
-- 
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: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080428/c3ee5740/attachment.pgp>



More information about the ffmpeg-devel mailing list