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

[Michael Niedermayer]

On Mon, Apr 28, 2008 at 06:56:32PM +0200, Michael Niedermayer wrote:
> 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

Heres a proper patch for that:

Index: libavcodec/libgsm.c
===================================================================
--- libavcodec/libgsm.c	(revision 13005)
+++ libavcodec/libgsm.c	(working copy)
@@ -41,18 +41,14 @@
                avctx->channels);
         return -1;
     }
-    if (avctx->sample_rate != 8000) {
-        av_log(avctx, AV_LOG_ERROR, "Sample rate 8000Hz required for GSM, got %dHz\n",
-               avctx->sample_rate);
-        return -1;
+
+    if(avctx->codec->decode){
+        if(!avctx->channels)
+            avctx->channels= 1;
+
+        if(!avctx->sample_rate)
+            avctx->sample_rate= 8000;
     }
-    if (avctx->bit_rate != 13000 /* Official */ &&
-        avctx->bit_rate != 13200 /* Very common */ &&
-        avctx->bit_rate != 0 /* Unknown; a.o. mov does not set bitrate when decoding */ ) {
-        av_log(avctx, AV_LOG_ERROR, "Bitrate 13000bps required for GSM, got %dbps\n",
-               avctx->bit_rate);
-        return -1;
-    }
 
     avctx->priv_data = gsm_create();
-----
This also makes the raw gsm partially play (needs a parser to be correctly
playable)
Ill apply the patch above in 24h unless you object.

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

I count him braver who overcomes his desires than him who conquers his
enemies for the hardest victory is over self. -- 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/20080429/23769ad1/attachment.pgp>