[FFmpeg-devel] [PATCH] libavformat/mpegtsenc: adaptive alignment for teletext PES packets

[Marton Balint]

On Wed, 22 May 2019, Andreas Håkon wrote:

> Hi,
>
> Patch to generate aligned Teletext PES packets using the MPEG-TS muxer
> when the TS header contains data.
>

> The code that generates the PES packets for Teletext data aligns the PES packets
> with the boundaries of the TS packets. The strategy used is to insert padding
> stuff into the PES header. The size of this padding alignment currently has a
> fixed size of 36 bytes. And this value is correct when all of the Teletext
> TS packets have payload only. However, inserting some data into the TS header
> breaks this assumption. In this case, the Teletext PES packets are not aligned
> and the end of the PES packet is inserted into an additional TS packet with a
> large amount of TS padding.

What is the data that is inserted into the TS header? Can you give an 
example? Are you sure that it is allowed to generate such streams? 
Are you seeing such streams in the wild?

The DVB teletext spec is very strict about that PES header size and 0x24 
is hardcoded there.

https://www.etsi.org/deliver/etsi_en/300400_300499/300472/01.03.01_60/en_300472v010301p.pdf

So maybe you should use adaptation field stuffing instead to keep the 
alignment?

> 
> This patch calculates the size of the TS header in order to reduce the size of
> the PES padding and thus accommodate the PES packet to the TS boundaries.
> In this way no additional TS packets are produced.
> 
> Signed-off-by: Andreas Hakon <andreas.hakon at protonmail.com>
>  libavformat/mpegtsenc.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index fc0ea22..f1772ac 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -1328,8 +1328,11 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
>                          header_len += 3;
>              }
>              if (is_dvb_teletext) {
> -                pes_header_stuffing_bytes = 0x24 - header_len;
> -                header_len = 0x24;
> +                int header_stuff = (get_ts_payload_start(buf) - buf) - 4;

This is a very bad variable name. Call it adaptation_field_len.

> +                if (header_stuff - header_len > 0x24)
> +                    header_stuff = 0;

Are you sure about this check?

> +                pes_header_stuffing_bytes = 0x24 - header_len - header_stuff;
> +                header_len = 0x24 - header_stuff;
>              }
>              len = payload_size + header_len + 3;
>              /* 3 extra bytes should be added to DVB subtitle payload: 0x20 0x00 at the beginning and trailing 0xff */
> --
> 1.7.10.4

Regards,
Marton