Created attachment 161655 [details] [review]
Patch that fixes the issue

Created attachment 161656 [details] [review]
Patch that fixes the issue

This seems to be some kind of enhanced, but backwards-compatible mp3 type?

Could you make a sample file available please?

audio/mpeg,mpegversion=2 doesn't seem right, we use that already for AAC / MPEG-2 part 7 IIRC.

This is the related part in the spec ISO/IEC 14496-1:

8.6.6.2   Semantics
The DecoderConfigDescriptor provides information about the decoder type and the required decoder
resources needed for the associated elementary stream. This is needed at the receiving terminal to determine
whether it is able to decode the elementary stream. A stream type identifies the category of the stream while the
optional decoder specific information descriptor contains stream specific information for the set up of the decoder in
a stream specific format that is opaque to this layer.
ObjectTypeIndication – an indication of the object or scene description type that needs to be supported by
the decoder for this elementary stream as per Table 8. For streamType values other than audioStream and
visualStream, the objectTypeIndication shall be set to 0xFF, indicating that no object type is specified.
                                       Table 8 - objectTypeIndication Values
                Value                ObjectTypeIndication Description
                0x00                 Forbidden
                                                                 a
                0x01                 Systems ISO/IEC 14496-1
                                                                 b
                0x02                 Systems ISO/IEC 14496-1
                0x03-0x1F            reserved for ISO use
                                                               c
                0x20                 Visual ISO/IEC 14496-2
                0x21-0x3F            reserved for ISO use
                                                               d
                0x40                 Audio ISO/IEC 14496-3
                0x41-0x5F            reserved for ISO use
                0x60                 Visual ISO/IEC 13818-2 Simple Profile
                0x61                 Visual ISO/IEC 13818-2 Main Profile
                0x62                 Visual ISO/IEC 13818-2 SNR Profile
                0x63                 Visual ISO/IEC 13818-2 Spatial Profile
                0x64                 Visual ISO/IEC 13818-2 High Profile
                0x65                 Visual ISO/IEC 13818-2 422 Profile
                0x66                 Audio ISO/IEC 13818-7 Main Profile
                0x67                 Audio ISO/IEC 13818-7 LowComplexity Profile
                0x68                 Audio ISO/IEC 13818-7 Scaleable Sampling Rate Profile
                0x69                 Audio ISO/IEC 13818-3
                0x6A                 Visual ISO/IEC 11172-2
                0x6B                 Audio ISO/IEC 11172-3
                0x6C                 Visual ISO/IEC 10918-1
                0x6D - 0xBF          reserved for ISO use
                0xC0 - 0xFE          user private
                0xFF                 no object type specified

As had been talked on IRC the patch isn't suficient, in terms of gstreamer caps we are currently using:

mpegversion = 1 layer {1,2,3} for all variants of MP3

and

mpegversion = {2, 4} for AAC

In ISO/IEC 13818-7:

8.1.4   Audio Data Transport Stream ADTS
The Audio Data Transport Stream (ADTS) is similar to syntax used in ISO/IEC 11172-3 and
ISO/IEC 13818-3. This will be recognized by ISO/IEC 11172-3 decoders as a “Layer 4”
bitstream.
The fixed header of the ADTS contains the syncword plus all parts of the header which are
necessary for decoding and which do not change from frame to frame. The variable header
of the ADTS contains header data which changes from frame to frame.

Maybe we should have caps for the following:

MP3: mpegversion = 1 layer = {1,2,3} and mpegversion = 2 layer = 3
AAC: mpegversion = 2 layer = 4 and mpegversion = 4

The main reason is correctness on typefinding.

I've spotted that mpegaudioparse is creating caps like the following:
audio/mpeg, mpegversion=(int)1, mpegaudioversion=(int)2, layer=(int)3

could be created like this in qtdemux too considering the objectTypeIndication value ?

Yes, ISO/IEC 13818-3 is mpegversion=1,mpegaudioversion=2. But it can also have three different layers, see http://www.mp3-tech.org/programmer/frame_header.html

The mp4/qt spec doesn't seem to specify the layer unfortunately. Anyway, do you want to provide an updated patch?

Josep: ping?

To properly has a correct disambiguation between MP3 and AAC, and the different standards we should change the caps schema so move it for 2.0.


I'll try to work on this at some point in the future if I don't miss it again.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/30.