[FFmpeg-devel] [PATCH] ac3dec_fixed: always use the USE_FIXED=1 variant of the AC3DecodeContext

[Michael Niedermayer]

On Fri, Mar 13, 2015 at 11:27:22PM +0100, Andreas Cadhalpun wrote:
> Hi,
> 
> On 13.03.2015 22:46, Christophe Gisquet wrote:
> > 2015-03-13 22:28 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun at googlemail.com>:
> >> -int ff_eac3_parse_header(AC3DecodeContext *s);
> >> +static int ff_eac3_parse_header(AC3DecodeContext *s);
> > 
> > It's somewhat cosmetics, but if these functions become static, they
> > would better drop the ff_ prefix.
> 
> I don't mind the names, but I tried to keep the changes minimal, to
> ease backporting the fix.
> The names can be changed in a follow-up patch.
> 
> >> -            float accum = 0.0f;
> >> +            INTFLOAT accum = 0.0f;
> > [...]
> >>              for (i = 0; i < bandsize; i++) {
> >> -                float coeff = s->transform_coeffs[ch][bin++];
> >> +                INTFLOAT coeff = s->transform_coeffs[ch][bin++];
> >>                  accum += coeff * coeff;
> > [...]
> >> -            float nscale = s->spx_noise_blend[ch][bnd] * rms_energy[bnd] * (1.0f / INT32_MIN);
> >> -            float sscale = s->spx_signal_blend[ch][bnd];
> >> +            INTFLOAT nscale = s->spx_noise_blend[ch][bnd] * rms_energy[bnd] * (1.0f / INT32_MIN);
> >> +            INTFLOAT sscale = s->spx_signal_blend[ch][bnd];
> >>              for (i = 0; i < s->spx_band_sizes[bnd]; i++) {
> >> -                float noise  = nscale * (int32_t)av_lfg_get(&s->dith_state);
> >> +                INTFLOAT noise  = nscale * (int32_t)av_lfg_get(&s->dith_state);
> > 
> > Does that work at all? I mean, if it's fixedpoint, I would have
> > expected some renormalization, various things to avoid overflows,
> > potential warnings because of casts, etc.
> > 
> > And is the output on eac3 samples, eg here:
> > http://samples.mplayerhq.hu/A-codecs/AC3/eac3/
> > (particularly the spx ones)
> > anything listenable?
> > 
> > Maybe the extensions should simply be declared as unsupported by the fp version?
> 
> It works well enough, I think.
> Without this patch:
> $ ffmpeg -c:a ac3_fixed -i ./rio_bravo_mono_64_spx.ac3 broken.ac3
> [eac3 @ 0x2401560] Estimating duration from bitrate, this may be inaccurate
> Input #0, eac3, from '/tmp/rio_bravo_mono_64_spx.ac3':
>   Duration: 00:04:22.18, start: 0.000000, bitrate: 64 kb/s
>     Stream #0:0: Audio: ac3, 48000 Hz, mono, s16p, 64 kb/s
> Output #0, ac3, to '/tmp/broken.ac3':
>   Metadata:
>     encoder         : Lavf56.25.101
>     Stream #0:0: Audio: ac3, 48000 Hz, mono, fltp, 96 kb/s
>     Metadata:
>       encoder         : Lavc56.26.100 ac3
> Stream mapping:
>   Stream #0:0 -> #0:0 (ac3 (ac3_fixed) -> ac3 (native))
> Press [q] to stop, [?] for help
> [ac3_fixed @ 0x24024c0] exponent out-of-range
> [ac3_fixed @ 0x24024c0] error decoding the audio block
> ...
> many more such errors
> ...
> [ac3_fixed @ 0x24024c0] exponent out-of-range
> [ac3_fixed @ 0x24024c0] error decoding the audio block
> Segmentation fault (core dumped)
> 
> 
> Make sure the volume is at a low level, when playing the
> broken.ac3, or your ears will hurt.
> 
> With this patch:
> $ ffmpeg -c:a ac3_fixed -i ./rio_bravo_mono_64_spx.ac3 out.ac3
> [eac3 @ 0xb5b560] Estimating duration from bitrate, this may be inaccurate
> Input #0, eac3, from '/tmp/rio_bravo_mono_64_spx.ac3':
>   Duration: 00:04:22.18, start: 0.000000, bitrate: 64 kb/s
>     Stream #0:0: Audio: ac3, 48000 Hz, mono, s16p, 64 kb/s
> Output #0, ac3, to '/tmp/out.ac3':
>   Metadata:
>     encoder         : Lavf56.25.101
>     Stream #0:0: Audio: ac3, 48000 Hz, mono, fltp, 96 kb/s
>     Metadata:
>       encoder         : Lavc56.26.100 ac3
> Stream mapping:
>   Stream #0:0 -> #0:0 (ac3 (ac3_fixed) -> ac3 (native))
> Press [q] to stop, [?] for help
> size=    3072kB time=00:04:22.17 bitrate=  96.0kbits/s    
> video:0kB audio:3072kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 0.000000%
> 
> The out.ac3 sounds much like the input.

for the case i tested the fixed and float differed by stddev ~ 500
fixed that and applied stddev is ~8 now

i think theres something wrong with sscale though as its always 0
even for float

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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150314/ae4ea504/attachment.asc>