Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#1890 closed defect (invalid)

avpriv_mpeg4audio_sample_rates vector size not correct

Reported by: Filippo Della Betta 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 (6)

comment:1 by Carl Eugen Hoyos, 11 years ago

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.

in reply to:  1 comment:2 by Filippo Della Betta, 11 years ago

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 by Carl Eugen Hoyos, 11 years ago

Resolution: invalid
Status: newclosed

Wouldn't that be 100% equivalent to the current code?

in reply to:  3 comment:4 by Filippo Della Betta, 11 years ago

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 by Clément Bœsch, 11 years ago

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).

in reply to:  5 comment:6 by Filippo Della Betta, 11 years ago

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!

Note: See TracTickets for help on using tickets.