Ticket #1890 (closed defect: invalid)
avpriv_mpeg4audio_sample_rates vector size not correct
| Reported by: | dellabetta | Owned by: | |
|---|---|---|---|
| Priority: | normal | Component: | avcodec |
| Version: | git-master | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Reproduced by developer: | no | |
| Analyzed by developer: | no |
Description
in mpeg4audio.c
const int avpriv_mpeg4audio_sample_rates[16] = {
96000, 88200, 64000, 48000, 44100, 32000,
24000, 22050, 16000, 12000, 11025, 8000, 7350
};
but there are only 13 values!
in aacenc.c, the code
for (i = 0; i < 16; i++)
if (avctx->sample_rate == avpriv_mpeg4audio_sample_rates[i])
break;
seems faulty.
IMHO avpriv_mpeg4audio_sample_rates should become
const int avpriv_mpeg4audio_sample_rates[13] = {
96000, 88200, 64000, 48000, 44100, 32000,
24000, 22050, 16000, 12000, 11025, 8000, 7350
};
and in aacenc.c you should safely write something like
for (i = 0; i < FF_ARRAY_ELEMS(avpriv_mpeg4audio_sample_rates); i++)
...
Change History
comment:2 in reply to: ↑ 1 Changed 7 months ago by dellabetta
Replying to cehoyos:
Please send patches to ffmpeg-devel, they receive more attention there.
But note that your change would (for example) complicate the code in libavcodec/aacadtsdec.c, so I am not sure the change is desirable.
I see, proabably the following change to mpeg4audio.c should be safer and easier
const int avpriv_mpeg4audio_sample_rates[16] = {
96000, 88200, 64000, 48000, 44100, 32000,
24000, 22050, 16000, 12000, 11025, 8000, 7350, 0, 0, 0
};
comment:3 follow-up: ↓ 4 Changed 7 months ago by cehoyos
- Status changed from new to closed
- Resolution set to invalid
Wouldn't that be 100% equivalent to the current code?
comment:4 in reply to: ↑ 3 Changed 7 months ago by dellabetta
Replying to cehoyos:
Wouldn't that be 100% equivalent to the current code?
No, it isn't IMHO. It depends on the compiler. Nobody guarantees that memory is initialized to 0.
comment:5 follow-up: ↓ 6 Changed 7 months ago by ubitux
It's guaranteed to be set to 0 by the C standard in this case. And we use that feature all around the code base.
Also, FF_ARRAY_ELEMS will not work as expected given that this array is exported (which is related to another real issue though).
comment:6 in reply to: ↑ 5 Changed 7 months ago by dellabetta
Replying to ubitux:
It's guaranteed to be set to 0 by the C standard in this case. And we use that feature all around the code base.
Also, FF_ARRAY_ELEMS will not work as expected given that this array is exported (which is related to another real issue though).
Thank you for the explanation!



Please send patches to ffmpeg-devel, they receive more attention there.
But note that your change would (for example) complicate the code in libavcodec/aacadtsdec.c, so I am not sure the change is desirable.