[FFmpeg-devel,5/6] avformat/mpegtsenc: fix PCR generation intervals

Submitted by Marton Balint on Aug. 8, 2019, 9:27 p.m.

Details

Message ID 20190808212753.29467-5-cus@passwd.hu
State New
Headers show

Commit Message

Marton Balint Aug. 8, 2019, 9:27 p.m.
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.

The accuracy of PCR packets for CBR streams was greatly improved by preemtively
sending them at PCR intervals even if sending the payload of another stream
is in progress.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/muxers.texi         |  4 +--
 libavformat/mpegtsenc.c | 92 ++++++++++++++++++++++---------------------------
 tests/ref/lavf/ts       |  2 +-
 3 files changed, 44 insertions(+), 54 deletions(-)

Comments

Andreas Håkon Aug. 9, 2019, 7:48 a.m.
Hi Marton,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, 8 de August de 2019 23:27, Marton Balint <cus@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.
>
> The accuracy of PCR packets for CBR streams was greatly improved by preemtively
> sending them at PCR intervals even if sending the payload of another stream
> is in progress.
>
> Signed-off-by: Marton Balint cus@passwd.hu

[...]

This new version LGTM.
Regards.
A.H.

---
Marton Balint Aug. 10, 2019, 8:25 p.m.
On Fri, 9 Aug 2019, Andreas Håkon wrote:

> Hi Marton,
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Thursday, 8 de August de 2019 23:27, Marton Balint <cus@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.
>>
>> The accuracy of PCR packets for CBR streams was greatly improved by preemtively
>> sending them at PCR intervals even if sending the payload of another stream
>> is in progress.
>>
>> Signed-off-by: Marton Balint cus@passwd.hu
>
> [...]
>
> This new version LGTM.

Thanks. I did a slight change for VBR streams and tried to mimic the old 
behaviour when selecting a PCR interval. Will post an updated version, but 
that should only affect VBR.

Regards,
Marton

Patch hide | download patch | download mbox

diff --git a/doc/muxers.texi b/doc/muxers.texi
index bc38cf6029..f9e8b60517 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1623,8 +1623,8 @@  is @code{-1}, which results in shifting timestamps so that they start from 0.
 Omit the PES packet length for video packets. Default is @code{1} (true).
 
 @item pcr_period @var{integer}
-Override the default PCR retransmission time in milliseconds. Ignored if
-variable muxrate is selected. Default is @code{20}.
+Override the default PCR retransmission time in milliseconds. Default is
+@code{20}.
 
 @item pat_period @var{double}
 Maximum time in seconds between PAT/PMT tables.
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index f4c1bb4717..aec2559b14 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -84,6 +84,7 @@  typedef struct MpegTSWrite {
     int onid;
     int tsid;
     int64_t first_pcr;
+    int64_t next_pcr;
     int mux_rate; ///< set to 1 when VBR
     int pes_payload_size;
 
@@ -237,10 +238,9 @@  typedef struct MpegTSWriteStream {
     int payload_flags;
     uint8_t *payload;
     AVFormatContext *amux;
-    AVRational user_tb;
 
-    int pcr_packet_count;
-    int pcr_packet_period;
+    int64_t pcr_period; /* PCR period in PCR time base */
+    int64_t last_pcr;
 
     /* For Opus */
     int opus_queued_samples;
@@ -787,39 +787,6 @@  fail:
     return NULL;
 }
 
-static void enable_pcr_generation_for_stream(AVFormatContext *s, AVStream *pcr_st)
-{
-    MpegTSWrite *ts = s->priv_data;
-    MpegTSWriteStream *ts_st = pcr_st->priv_data;
-
-    if (ts->mux_rate > 1) {
-        ts_st->pcr_packet_period   = (int64_t)ts->mux_rate * ts->pcr_period /
-                                     (TS_PACKET_SIZE * 8 * 1000);
-    } else {
-        if (pcr_st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
-            int frame_size = av_get_audio_frame_duration2(pcr_st->codecpar, 0);
-            if (!frame_size) {
-                av_log(s, AV_LOG_WARNING, "frame size not set\n");
-                ts_st->pcr_packet_period =
-                    pcr_st->codecpar->sample_rate / (10 * 512);
-            } else {
-                ts_st->pcr_packet_period =
-                    pcr_st->codecpar->sample_rate / (10 * frame_size);
-            }
-        } else {
-            // max delta PCR 0.1s
-            // TODO: should be avg_frame_rate
-            ts_st->pcr_packet_period =
-                ts_st->user_tb.den / (10 * ts_st->user_tb.num);
-        }
-        if (!ts_st->pcr_packet_period)
-            ts_st->pcr_packet_period = 1;
-    }
-
-    // output a PCR as soon as possible
-    ts_st->pcr_packet_count = ts_st->pcr_packet_period;
-}
-
 static void select_pcr_streams(AVFormatContext *s)
 {
     MpegTSWrite *ts = s->priv_data;
@@ -842,8 +809,11 @@  static void select_pcr_streams(AVFormatContext *s)
         if (pcr_st) {
             MpegTSWriteStream *ts_st = pcr_st->priv_data;
             service->pcr_pid = ts_st->pid;
-            enable_pcr_generation_for_stream(s, pcr_st);
-            av_log(s, AV_LOG_VERBOSE, "service %i using PCR in pid=%i\n", service->sid, service->pcr_pid);
+            ts_st->pcr_period = av_rescale(ts->pcr_period, PCR_TIME_BASE, 1000);
+            // output a PCR as soon as possible
+            ts_st->last_pcr   = ts->first_pcr - ts_st->pcr_period;
+            av_log(s, AV_LOG_VERBOSE, "service %i using PCR in pid=%i, pcr_period=%"PRId64"ms\n",
+                service->sid, service->pcr_pid, av_rescale(ts_st->pcr_period, 1000, PCR_TIME_BASE));
         }
     }
 }
@@ -907,7 +877,6 @@  static int mpegts_init(AVFormatContext *s)
         }
         st->priv_data = ts_st;
 
-        ts_st->user_tb = st->time_base;
         avpriv_set_pts_info(st, 33, 1, 90000);
 
         ts_st->payload = av_mallocz(ts->pes_payload_size);
@@ -1183,7 +1152,6 @@  static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
     uint8_t *q;
     int val, is_start, len, header_len, write_pcr, is_dvb_subtitle, is_dvb_teletext, flags;
     int afc_len, stuffing_len;
-    int64_t pcr = -1; /* avoid warning */
     int64_t delay = av_rescale(s->max_delay, 90000, AV_TIME_BASE);
     int force_pat = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && key && !ts_st->prev_payload_key;
 
@@ -1194,16 +1162,42 @@  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;
+        if (ts->mux_rate > 1) {
+            /* Send PCR packets for all PCR streams if needed */
+            pcr = get_pcr(ts, s->pb);
+            if (pcr >= ts->next_pcr) {
+                int64_t next_pcr = INT64_MAX;
+                for (int i = 0; i < s->nb_streams; i++) {
+                    /* Make the current stream the last, because for that we
+                     * can insert the pcr into the payload later */
+                    int st2_index = i < st->index ? i : (i + 1 == s->nb_streams ? st->index : i + 1);
+                    AVStream *st2 = s->streams[st2_index];
+                    MpegTSWriteStream *ts_st2 = st2->priv_data;
+                    if (ts_st2->pcr_period) {
+                        if (pcr - ts_st2->last_pcr >= ts_st2->pcr_period) {
+                            ts_st2->last_pcr = FFMAX(pcr - ts_st2->pcr_period, ts_st2->last_pcr + ts_st2->pcr_period);
+                            if (st2 != st) {
+                                mpegts_insert_pcr_only(s, st2);
+                                pcr = get_pcr(ts, s->pb);
+                            } else {
+                                write_pcr = 1;
+                            }
+                        }
+                        next_pcr = FFMIN(next_pcr, ts_st2->last_pcr + ts_st2->pcr_period);
+                    }
+                }
+                ts->next_pcr = next_pcr;
+            }
+        } else if (ts_st->pcr_period && dts != AV_NOPTS_VALUE) {
+            pcr = (dts - delay) * 300;
+            if (pcr - ts_st->last_pcr >= ts_st->pcr_period && is_start) {
+                ts_st->last_pcr = FFMAX(pcr - ts_st->pcr_period, ts_st->last_pcr + ts_st->pcr_period);
                 write_pcr = 1;
             }
         }
@@ -1236,7 +1230,7 @@  static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
         }
         if (key && is_start && pts != AV_NOPTS_VALUE) {
             // set Random Access for key frames
-            if (ts_st->pcr_packet_period)
+            if (ts_st->pcr_period)
                 write_pcr = 1;
             set_af_flag(buf, 0x40);
             q = get_ts_payload_start(buf);
@@ -1245,10 +1239,6 @@  static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
             set_af_flag(buf, 0x10);
             q = get_ts_payload_start(buf);
             // add 11, pcr references the last byte of program clock reference base
-            if (ts->mux_rate > 1)
-                pcr = get_pcr(ts, s->pb);
-            else
-                pcr = (dts - delay) * 300;
             if (dts != AV_NOPTS_VALUE && dts < pcr / 300)
                 av_log(s, AV_LOG_WARNING, "dts < pcr, TS is invalid\n");
             extend_af(buf, write_pcr_bits(q, pcr));
diff --git a/tests/ref/lavf/ts b/tests/ref/lavf/ts
index 09960f84d3..8644c260e3 100644
--- a/tests/ref/lavf/ts
+++ b/tests/ref/lavf/ts
@@ -1,3 +1,3 @@ 
-38f4b14d43a0e416be8d598628997cbc *tests/data/lavf/lavf.ts
+3eee8c2ef073ecdb2b85f40bb9b2e652 *tests/data/lavf/lavf.ts
 407020 tests/data/lavf/lavf.ts
 tests/data/lavf/lavf.ts CRC=0x71287e25