Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#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 (6)

comment:1 follow-up: Changed 5 years ago by 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.

comment:2 in reply to: ↑ 1 Changed 5 years 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: Changed 5 years ago by cehoyos

  • Resolution set to invalid
  • Status changed from new to closed

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

comment:4 in reply to: ↑ 3 Changed 5 years 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: Changed 5 years 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 5 years 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!

Note: See TracTickets for help on using tickets.