[FFmpeg-devel] [PATCH v3 3/3] avformat/mpegtsenc: Write stream_id into PES after stream_id decision

[Mao Hata]

On 2021/04/22 21:03, zheng qian wrote:
> Changes since v2:
>    Fix PES_packet_length mismatch bug
>
> According to the PES packet definition defined in Table 2-17
> of ISO_IEC_13818-1 specification, some fields like PTS/DTS or
> pes_extension could only appears if the stream_id meets the
> condition:
>
> if (stream_id != 0xBC &&  // program_stream_map
> stream_id != 0xBE &&  // padding_stream
> stream_id != 0xBF &&  // private_stream_2
> stream_id != 0xF0 &&  // ECM
> stream_id != 0xF1 &&  // EMM
> stream_id != 0xFF &&  // program_stream_directory
> stream_id != 0xF2 &&  // DSMCC_stream
> stream_id != 0xF8)     // ITU-T Rec. H.222.1 type E stream
>
> And the following stream_id types don't have fields like PTS/DTS:
>
> else if ( stream_id == program_stream_map
> || stream_id == private_stream_2
> || stream_id == ECM
> || stream_id == EMM
> || stream_id == program_stream_directory
> || stream_id == DSMCC_stream
> || stream_id == ITU-T Rec. H.222.1 type E stream ) {
>      for (i = 0; i < PES_packet_length; i++) {
>          PES_packet_data_byte
>      }
> }
>
> Current implementation skipped the judgement of stream_id
> causing some kind of stream like private_stream_2 to be
> incorrectly written with PTS/DTS field. For example,
> Japan DTV transmits news and alerts through ARIB superimpose
> that utilizes private_stream_2 still could not be remuxed
> correctly for now.
>
> This patch set fixes the remuxing for private_stream_2 and
> other stream_id types.
>
> Signed-off-by: zheng qian <xqq at xqq.im>
> ---
>   libavformat/mpegtsenc.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index f302af84ff..b59dab5174 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -1476,6 +1476,16 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
>                   }
>               }
>               header_len = 0;
> +
> +            if (stream_id != 0xBC &&  // program_stream_map
> +                stream_id != 0xBE &&  // padding_stream
> +                stream_id != 0xBF &&  // private_stream_2
> +                stream_id != 0xF0 &&  // ECM
> +                stream_id != 0xF1 &&  // EMM
> +                stream_id != 0xFF &&  // program_stream_directory
> +                stream_id != 0xF2 &&  // DSMCC_stream
> +                stream_id != 0xF8) {  // ITU-T Rec. H.222.1 type E stream > +
>               flags      = 0;
>               if (pts != AV_NOPTS_VALUE) {
>                   header_len += 5;
> @@ -1569,6 +1579,11 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
>                   memset(q, 0xff, pes_header_stuffing_bytes);
>                   q += pes_header_stuffing_bytes;
>               }
> +            } else {
> +                len = payload_size;
> +                *q++ = len >> 8;
> +                *q++ = len;
> +            }
>               is_start = 0;
>           }
>           /* header size */
>

Your patch changes the behavior of mpegts_write_pes() function only if 
its "st->codecpar->codec_type" parameter is AVMEDIA_TYPE_DATA. In other 
cases, "stream_id" variable in this function is set to any of the following:
STREAM_ID_EXTENDED_STREAM_ID (0xfd)
STREAM_ID_VIDEO_STREAM_0 (0xe0)
STREAM_ID_AUDIO_STREAM_0 (0xc0)
STREAM_ID_EXTENDED_STREAM_ID (0xfd)
STREAM_ID_PRIVATE_STREAM_1 (0xbd)
So, the "if (stream_id != ...)" statement added by the patch is always 
true, therefore, the function behaves the same as before.

If "st->codecpar->codec_type" parameter is AVMEDIA_TYPE_DATA, unless 
additionally conditioned by "st->codecpar->codec_id" parameter, 
basically, the "stream_id" value given as an argument is written to PES 
packets as it is. In this case, the behavior of the function should be 
fixed as you commented.

v3 patch looks good to me.
Then, since the patch itself is based on ISO_IEC_13818-1, it perhaps has 
some (side) effects for ATSC/DVB as well. If possible, you might want to 
wait for opinions from ATSC/DVB sides.

Mao Hata

P.S.
I confirmed the effect of your patch with the following command.
$ ffmpeg -i some_arib_mpeg.ts -c:v libx264 -map 0:v -c:a aac -map 0:a 
-map 0:4 dest.ts

Here "-map 0:4" points to private_stream_2 (ARIB-superimpose).
The command now properly remuxes "-map 0:4" stream into "dest.ts" 
without any editing.