[FFmpeg-devel] [PATCH 1/3] avformat/mpegtsenc: use total_size instead of avio_tell()
lance.lmwang at gmail.com
lance.lmwang at gmail.com
Thu Oct 22 04:29:13 EEST 2020
On Wed, Oct 21, 2020 at 11:39:10PM +0200, Marton Balint wrote:
>
>
> On Wed, 21 Oct 2020, lance.lmwang at gmail.com wrote:
>
> > From: Limin Wang <lance.lmwang at 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.
Thanks, Will update the commit log with yours, it's more clear.
>
> >
> > 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.lmwang at 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 at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
--
Thanks,
Limin Wang
More information about the ffmpeg-devel
mailing list