[FFmpeg-devel] [PATCH] n-th try to get the MPEG-PS demuxer functional

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> Hi
>
> Patch below fixes issue49
> Departed Trailer.evo (mpeg ps missdetecting h264 as mpeg2video)
> (and possibly others)
>
> I also would like to emphasize that mans claim of spec non compliance does
> not seem to be true. The PSM is not a mandatory element in MPEG-PS not even
> for H.264 in MPEG-PS
>
> ---
> 2.14.2    Carriage in PES packets
> ITU-T Rec. H.264 | ISO/IEC 14496-10 Video is carried in PES packets
> as PES_packet_data_bytes, using one of the 16 stream_id values
> assigned to video, while signalling the ITU-T Rec. H.264 | ISO/IEC
> 14496-10 Video stream by means of the assigned stream-type value in
> the PMT or PSM (see Table 2-34). The highest level that may occur in
> an AVC video stream as well as a profile that the entire stream
> conforms to should be signalled using the AVC video descriptor. If
> an AVC video descriptor is associated with an AVC video stream, then
> this descriptor shall be conveyed in the descriptor loop for the
> respective elementary stream entry in the Program Map Table in case
> of a Transport Stream or in the Program Stream Map, when PSM is
>                                                     ^^^^^^^^^^^
> present, in case of a Program Stream. This Recommendation |
> ^^^^^^^
> International Standard does not specify presentation of ITU-T
> Rec. H.264 | ISO/IEC 14496-10 streams in the context of a program.
> ---
>
> The underlined part makes it clear that a PSM is not mandatory.

I never said PSM was mandatory.  It is of course entirely optional.

> ---
>                Table 2-22  Stream_id assignments
> Stream_id Note                                   stream coding
> 1011 1100  1     program_stream_map
> 1011 1101  2     private_stream_1
> 1011 1110        padding_stream
> 1011 1111  3     private_stream_2
> 110x xxxx        ISO/IEC 13818-3 or ISO/IEC 11172-3 or ISO/IEC 13818-7 or ISO/IEC 14496-3
>                  audio stream number x xxxx
> 1110 xxxx        ITU-T Rec. H.262 | ISO/IEC 13818-2, ISO/IEC 11172-2, ISO/IEC 14496-2
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                  or ITU-T Rec. H.264 | ISO/IEC 14496-10 video stream number xxxx
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 1111 0000  3     ECM_stream
> 1111 0001  3     EMM_stream
> 1111 0010  5     ITU-T Rec. H.222.0 | ISO/IEC 13818-1 Annex A
>                  or ISO/IEC 13818-6_DSMCC_stream
> 1111 0011  2     ISO/IEC_13522_stream
> 1111 0100  6     ITU-T Rec. H.222.1 type A
> ----
>
> And this part makes it clear that the ids are NOT reserved for
> mpeg1/mpeg2 video. They likely where reserved for mpeg1/2 in early
> versions of the mpeg2 spec which predated other video standards, but
> these have been withdrawn or are superseded by above, which inlcudes
> a dozen video coding standards.
>
> Thus summary, MPEG-PS does not contain any mandatory element to
> identify the used codec.

Yes, this is correct.  I never said otherwise.

You are probably confusing this with damaged MPEG-TS streams without
PAT or PMT.  Those elements *are* required.

> My patch corrects this by removing the claim that stream_id=1110xxxx
> would be mpeg2video. It thus makes our demuxer comply more with the
> spec not less.

What I've rejected in the past is patches assuming stream_id=e1 would
be H.264 (or whatever it was).  Such assignments may well be typical
(or even required) on HD-DVD (or other MPEG-PS derivatives), but this
does not preclude these IDs being used for other codecs in plain
MPEG-PS streams.

> Index: libavformat/mpeg.c
> ===================================================================
> --- libavformat/mpeg.c	(revision 14080)
> +++ libavformat/mpeg.c	(working copy)
> @@ -459,7 +459,7 @@
>          if(!memcmp(buf, avs_seqh, 4) && (buf[6] != 0 || buf[7] != 1))
>              codec_id = CODEC_ID_CAVS;
>          else
> -            codec_id = CODEC_ID_MPEG2VIDEO;
> +            codec_id = CODEC_ID_PROBE;
>          type = CODEC_TYPE_VIDEO;
>      } else if (startcode >= 0x1c0 && startcode <= 0x1df) {
>          type = CODEC_TYPE_AUDIO;

Patch OK.  That said, wouldn't it be better to support CAVS through
the probe functionality too?

-- 
M?ns Rullg?rd
mans at mansr.com