[FFmpeg-devel] [PATCH] fix swf playback with bad audio track

[Baptiste Coudurier]

Aurelien Jacobs wrote:
> On Tue, 11 Sep 2007 14:04:40 +0200
> Baptiste Coudurier <baptiste.coudurier at smartjog.com> wrote:
> 
> 
>>Hi
>>
>>Reimar D?ffinger wrote:
>>
>>>Hello,
>>>On Tue, Sep 11, 2007 at 11:24:15AM +0200, Aurelien Jacobs wrote:
>>>
>>>>On Tue, 11 Sep 2007 01:20:23 +0200
>>>>Baptiste Coudurier <baptiste.coudurier at smartjog.com> wrote:
>>>>
>>>>>Aurelien Jacobs wrote:
>>>>>
>>>>>>Right now, when the swf demuxer find an audio track with unrecognized
>>>>>>samplerate, it simply return AVERROR(EIO). This prevent playing the
>>>>>>video track.
>>>>>
>>>>>It seems weird, samplerate should not be "unrecognized", Im curious, is
>>>>>such a file available somewhere ?
>>>>
>>>>To be more precise, sample_rate_code has 2 significant bits. Values
>>>>between 1 and 3 maps to samplerate 11025, 22050 and 44100. 
>>>>sample_rate_code == 0 don't map to any samplerate (at least in current
>>>>code).
>>>>Here is a sample which has sample_rate_code = 0:
>>>>  http://samples.mplayerhq.hu/SWF/962_fws.swf
>>>
>>>I don't quite get why it should fail at all, mp3 and probably most
>>>other audio codecs just don't need a sample rate from the container.
>>>IMO just replace
>>>
>>>
>>>>  if (!sample_rate_code)
>>>>      return AVERROR(EIO);
>>>>  ast->codec->sample_rate = 11025 << (sample_rate_code-1);
>>>
>>>by
>>>
>>>
>>>>  if (sample_rate_code)
>>>>      ast->codec->sample_rate = 11025 << (sample_rate_code-1);
>>>
>>>and change nothing else.
>>>
>>
>>No, according to specs 0 is 5.5khz sample rate. Code must be fixed
>>accordingly, and not failing.
> 
> 
> OK. First attached patch does this.
> 
> 
>>Now concerning the sample, it uses DEFINESOUND tag which is a block a
>>sound data, ie all mp3 frames are in one block, so demuxer must
>>interleave those frames with video frames. This is not supported atm.
> 
> 
> Second attached patch implement support for DEFINESOUND.
> Audio packetizing and interleaving is ugly, but at least it
> works for the sample mentioned in this tread.
> 
> Aurel
> 
> 
> ------------------------------------------------------------------------
> 
> Index: libavformat/swf.c
> ===================================================================
> --- libavformat/swf.c	(revision 10505)
> +++ libavformat/swf.c	(working copy)
> @@ -689,9 +689,7 @@
>              ast->codec->codec_id = codec_get_id(swf_audio_codec_tags, (v>>4) & 15);
>              ast->need_parsing = AVSTREAM_PARSE_FULL;
>              sample_rate_code= (v>>2) & 3;
> -            if (!sample_rate_code)
> -                return AVERROR(EIO);
> -            ast->codec->sample_rate = 11025 << (sample_rate_code-1);
> +            ast->codec->sample_rate = 44100 >> (3-sample_rate_code);
>              av_set_pts_info(ast, 64, 1, ast->codec->sample_rate);
>              len -= 4;
>          } else if (tag == TAG_VIDEOFRAME) {

This looks ok.

> [...]
>
> +            ast->codec->channels = 1 + (v&1);
> +            ast->codec->codec_type = CODEC_TYPE_AUDIO;
> +            ast->codec->codec_id = codec_get_id(swf_audio_codec_tags, v>>4);
> +            sample_rate_code= (v>>2) & 3;
> +            ast->codec->sample_rate = 44100 >> (3-sample_rate_code);
> +            av_set_pts_info(ast, 64, 1, ast->codec->sample_rate);

This is duplicate from code in TAG_STREAMHEAD/2. Can you add a new
function ?

Also ast might be null if no TAG_STREAMHEAD/2 is found before.

> [...]
> +            if (len > 100*1024*1024)
> +                return AVERROR(EIO);

I think EINVAL is better here.

> +            swf->audio_packet_size = len/(nb_samples/(ast->codec->sample_rate*256/swf->frame_rate));

division by 0 if nb_samples is 0.

And like michael pointed out, If codec is mp3, you need to set
AVSTREAM_PARSE_FULL when vbr, this might A/V sync problems I think.

This is ugly indeed. Maybe we could just output the big packet with
AVSTREAM_PARSE_FULL which will split out audio frames, and let the user
application correctly interleave video and audio frames based on dts.

Also lavf could do it in a generic way after parsing, It will take some
memory, though I see another application of that mechanism in other
demuxers (mov/mxf).

What do you guys think ?

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312