[FFmpeg-devel] [PATCH] avformat/mpegtsenc: fix PCR generation intervals

[Andreas Håkon]

Hi Marton,


‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, 4 de August de 2019 22:30, Marton Balint <cus at passwd.hu> wrote:

> PCR generation was based on counting packets for both CBR and VBR streams.
> Couting packets might have worked for CBR streams (when muxrate was specified)
> but it only took into account the packets of a service (or the packets of the
> PCR stream lately), so even that was problematic for multi program streams.
>
> The new code works on actual PCR for CBR and packet DTS values for VBR streams,
> so the default 20ms PCR retransmission time is now respected for both CBR and
> VBR.
>

I do some tests with this patch and the previous https://patchwork.ffmpeg.org/patch/14210/

And the result is that the repetition rate of PCR is inconsistent: it's never
constant and varies between different PCR streams.

But fortunately I have an idea...


> Signed-off-by: Marton Balint cus at passwd.hu
>
> libavformat/mpegtsenc.c | 60 +++++++++++++++----------------------------------
> tests/ref/lavf/ts | 2 +-
> 2 files changed, 19 insertions(+), 43 deletions(-)
>
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c

[...]

> @@ -1194,18 +1168,24 @@  static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
>
>      is_start = 1;
>      while (payload_size > 0) {
> +        int64_t pcr = -1; /* avoid warning */
> +
>          retransmit_si_info(s, force_pat, dts);
>          force_pat = 0;
>
>          write_pcr = 0;
> -        if (ts_st->pcr_packet_period) {
> -            if (ts->mux_rate > 1 || is_start) // VBR pcr period is based on frames
> -                ts_st->pcr_packet_count++;
> -            if (ts_st->pcr_packet_count >=
> -                ts_st->pcr_packet_period) {
> -                ts_st->pcr_packet_count = 0;
> -                write_pcr = 1;
> +        if (ts_st->pcr_period) {
> +            if (ts->mux_rate > 1) {
> +                pcr = get_pcr(ts, s->pb);
> +                if (pcr - ts_st->last_pcr >= ts_st->pcr_period)
> +                    write_pcr = 1;
> +            } else if (dts != AV_NOPTS_VALUE) {
> +                pcr = (dts - delay) * 300;
> +                if (pcr - ts_st->last_pcr >= ts_st->pcr_period && is_start)
> +                    write_pcr = 1;
>              }
> +            if (write_pcr)
> +                ts_st->last_pcr = FFMAX(pcr - ts_st->pcr_period, ts_st->last_pcr + ts_st->pcr_period);
>          }

IMHO, here you return to make the same mistake of the previous code:
only consider one PCR stream. Instead of the "if (ts_st->pcr_period)" it's
required to iterate over all PCR streams and do the corresponding checks.
And if some PCR stream has exceeded the pcr_period limit then enforce to
write an empty TS packet with a PCR mark.

In fact, the root cause is the sequential PES writing on the MPEG-TS muxer.
And this is the reason for my interleaving mux patch:
https://patchwork.ffmpeg.org/patch/13745/

However, until I rebase your code for the interleave mux mode, the PCR
generation of this patch needs to be fixed.

[...]


Regards.
A.H.

---