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>