On Wed, 21 Oct 2020, lance.lmw...@gmail.com wrote:

From: Limin Wang <lance.lmw...@gmail.com>

avio_tell() failed to get corrected data size in some condition.

Please explain. E.g. avio_tell() fails to get the amount of data written so far when the underlying IO context is flushed to segments.


Please test with below command:
$ ./ffmpeg  -f lavfi -i testsrc=duration=100:size=vga -c:v libx264 -x264opts \
 nal-hrd=cbr:force-cfr=1 -b:v 500k -minrate 500k -maxrate 500k -bufsize 500k \
 -f hls -hls_time 10  -hls_ts_options "muxrate=2000000" test%d.ts

$ du -h test*.ts
Before:
2.4M    test00.ts
4.8M    test11.ts
7.2M    test22.ts
9.6M    test33.ts
12M     test44.ts
15M     test55.ts
17M     test66.ts
20M     test77.ts
22M     test88.ts
24M     test99.ts

After apply the patch:
2.4M   test00.ts
2.4M   test11.ts
2.4M   test22.ts
2.4M   test33.ts
2.4M   test44.ts
2.4M   test55.ts
2.4M   test66.ts
2.4M   test77.ts
2.4M   test88.ts
2.4M   test99.ts
4.0K   test%d.ts

Signed-off-by: Limin Wang <lance.lmw...@gmail.com>
---
libavformat/mpegtsenc.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index afdadda..e05d0ce 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -84,6 +84,7 @@ typedef struct MpegTSWrite {
    int64_t next_pcr;
    int mux_rate; ///< set to 1 when VBR
    int pes_payload_size;
+    int64_t total_size;

    int transport_stream_id;
    int original_network_id;
@@ -830,9 +831,9 @@ invalid:
    return 0;
}

-static int64_t get_pcr(const MpegTSWrite *ts, AVIOContext *pb)
+static int64_t get_pcr(const MpegTSWrite *ts)
{
-    return av_rescale(avio_tell(pb) + 11, 8 * PCR_TIME_BASE, ts->mux_rate) +
+    return av_rescale(ts->total_size + 11, 8 * PCR_TIME_BASE, ts->mux_rate) +
           ts->first_pcr;
}

@@ -840,13 +841,15 @@ static void write_packet(AVFormatContext *s, const 
uint8_t *packet)
{
    MpegTSWrite *ts = s->priv_data;
    if (ts->m2ts_mode) {
-        int64_t pcr = get_pcr(s->priv_data, s->pb);
+        int64_t pcr = get_pcr(s->priv_data);
        uint32_t tp_extra_header = pcr % 0x3fffffff;
        tp_extra_header = AV_RB32(&tp_extra_header);
        avio_write(s->pb, (unsigned char *) &tp_extra_header,
                   sizeof(tp_extra_header));
+        ts->total_size += sizeof(tp_extra_header);

The PCR of M2TS should not be affected by the extra header, so please remove this addition. Also see my last comment.

    }
    avio_write(s->pb, packet, TS_PACKET_SIZE);
+    ts->total_size += TS_PACKET_SIZE;
}

static void section_write_packet(MpegTSSection *s, const uint8_t *packet)
@@ -987,6 +990,7 @@ static int mpegts_init(AVFormatContext *s)
        }
    }

+    ts->total_size = 0;

Unneeded initialization, please remove.

    if (s->max_delay < 0) /* Not set by the caller */
        s->max_delay = 0;

@@ -1228,7 +1232,7 @@ static void mpegts_insert_pcr_only(AVFormatContext *s, 
AVStream *st)
    }

    /* PCR coded into 6 bytes */
-    q += write_pcr_bits(q, get_pcr(ts, s->pb));
+    q += write_pcr_bits(q, get_pcr(ts));

    /* stuffing bytes */
    memset(q, 0xFF, TS_PACKET_SIZE - (q - buf));
@@ -1315,7 +1319,7 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream 
*st,
    while (payload_size > 0) {
        int64_t pcr = AV_NOPTS_VALUE;
        if (ts->mux_rate > 1)
-            pcr = get_pcr(ts, s->pb);
+            pcr = get_pcr(ts);
        else if (dts != AV_NOPTS_VALUE)
            pcr = (dts - delay) * 300;

@@ -1326,7 +1330,7 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream 
*st,
        write_pcr = 0;
        if (ts->mux_rate > 1) {
            /* Send PCR packets for all PCR streams if needed */
-            pcr = get_pcr(ts, s->pb);
+            pcr = get_pcr(ts);
            if (pcr >= ts->next_pcr) {
                int64_t next_pcr = INT64_MAX;
                for (int i = 0; i < s->nb_streams; i++) {
@@ -1340,7 +1344,7 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream 
*st,
                            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);
+                                pcr = get_pcr(ts);
                            } else {
                                write_pcr = 1;
                            }
@@ -1983,7 +1987,7 @@ static void mpegts_write_flush(AVFormatContext *s)
    }

    if (ts->m2ts_mode) {
-        int packets = (avio_tell(s->pb) / (TS_PACKET_SIZE + 4)) % 32;
+        int packets = (ts->total_size / (TS_PACKET_SIZE + 4)) % 32;

I'd rather keep avio_tell(s->pb) here, because I have a patch series which changes m2ts mode to use proper CBR muxing, and that must keep track the number of "fake" null packets written to maintain proper PCR:

https://github.com/cus/ffmpeg/commits/mpegts2

(Note that that series tracks the number of packets and not "total_size", but they are the same, so after you apply this patch that series can be easily adopted).

Thanks,
Marton

        while (packets++ < 32)
            mpegts_insert_null_packet(s);
    }
--
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to