[FFmpeg-devel] MPEG Audio elementary streams and layers
Tue Dec 16 16:51:57 CET 2008
On Tue, Dec 16, 2008 at 10:00:40AM +0100, Marc Mason wrote:
> Michael Niedermayer wrote:
> > Marc Mason wrote:
> >> CODEC_ID_MP2 and CODEC_ID_MP3 are defined in avcodec.h
> >> As far as I understand,
> >> CODEC_ID_MP2 = MPEG Audio Layer II
> >> CODEC_ID_MP3 = MPEG Audio Layer III
> >> CODEC_ID_MP3 appeared in rev 2231 with the following comment.
> >> /* preferred ID for MPEG Audio layer 1, 2 or 3 decoding */
> >> http://svn.ffmpeg.org/ffmpeg/trunk/libavcodec/avcodec.h?r1=2217&r2=2231
> >> What does the comment mean ?
> Does anybody remember what the comment means ?
svn blame will tell you who and when that comment was added ...
> >> ff_mpegaudio_decode_header() determines the layer of the ES.
> >> http://cekirdek.pardus.org.tr/~ismail/ffmpeg-docs/mpegaudiodecheader_8c-source.html#l00033
> >> s->layer = 4 - ((header >> 17) & 3);
> >> ff_mpa_decode_header() then copies the info to avctx->sub_id
> >> http://cekirdek.pardus.org.tr/~ismail/ffmpeg-docs/mpegaudio__parser_8c-source.html#l00047
> >> avctx->sub_id = s->layer;
> >> When av_find_stream_info() is given an MPEG Audio Layer II elementary
> >> stream, it returns CODEC_ID_MP3 with sub_id=2
> >> This seems ambiguous to me.
> >> What was the original intent ? How is one supposed to determine the
> >> layer of an MPEG Audio elementary stream ?
> >> I can see two possibilities.
> >> 1.) Use codec_id
> >> i.e. libavformat returns codec_id = one of CODEC_ID_MP1, CODEC_ID_MP2,
> >> CODEC_ID_MP3 according to the layer.
> >> Then ff_mpa_decode_header() should set the field to CODEC_ID_MP2 when
> >> appropriate.
> >> Something along the lines of
> >> $ svn diff
> >> Index: mpegaudio_parser.c
> >> ===================================================================
> >> --- mpegaudio_parser.c (revision 16054)
> >> +++ mpegaudio_parser.c (working copy)
> >> @@ -62,6 +62,7 @@
> >> break;
> >> case 2:
> >> avctx->frame_size = 1152;
> >> + avctx->codec_id = CODEC_ID_MP2;
> >> break;
> >> default:
> >> case 3:
> > I suspect this patch is ok given it is tested and the indention isn't off
> I suppose you meant "on condition that it is tested" ?
but looking again, the codec_id likely should be handled like the sample
rate, this should be more robust in presence of errors that might look like
> > and codec_id is also set for the MP3 case similarly
> AFAIU, mp3_read_header() initializes codec_id to CODEC_ID_MP3. But it
if mp3_read_header() has been called, that is not certain though
> won't hurt performance to set it again in ff_mpa_decode_header()
> >> However, what happens with layer I ES ?
> >> CODEC_ID_MP1 might have to be defined ?
> > yes
> > [...]
> You snipped the second option.
> Does that mean you prefer the first option ?
> In that case, setting sub_id becomes redundant, doesn't it ?
if you remove its only use from utils.c ...
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel