| Message ID | 4b9b1e3d-3d4e-a353-d037-b9d924c808d6@m1stereo.tv |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] Fix first_pcr initial update | expand |
| Context | Check | Description |
|---|---|---|
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/commit_msg_ppc | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Tue, Nov 02, 2021 at 12:47:57PM +0200, Maksym Veremeyenko wrote: > One of latest commit https://source.ffmpeg.org/?p=ffmpeg.git;a=commitdiff;h=6f36eb0da71d22aadf8f056f0966bd86656ea57e > claim it fixes endless loop on package generation if muxrate specified and > copyts used. But actually it does not work properly if *-mpegts_copyts 1* > specified: > > ffmpeg -y -copyts -i loewe.ts -c:v libx264 -x264opts nal-hrd=cbr:force-cfr=1 > -b:v 3500k -minrate 3500k -maxrate 3500k -bufsize 1000k -c:a mp2 -f mpegts > -mpegts_copyts 1 -muxrate 4500k -vframes 1000 test.ts > > ffmpeg generate huge file until it reach zero-based pcr value equal to first > dts. > > attached patch fix it. > > > -- > Maksym Veremeyenko > From fff0339c6d764e18fc4ecf111d035095309dc4d4 Mon Sep 17 00:00:00 2001 > From: Maksym Veremeyenko <verem@m1.tv> > Date: Tue, 2 Nov 2021 12:33:54 +0200 > Subject: [PATCH] Fix first_pcr initial update > > --- > libavformat/mpegtsenc.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c > index 35c835c..32786be 100644 > --- a/libavformat/mpegtsenc.c > +++ b/libavformat/mpegtsenc.c > @@ -1693,17 +1693,17 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt) > stream_id = side_data[0]; > > if (ts->copyts < 1) { > - if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { > - ts->first_pcr += dts * 300; > - ts->first_dts_checked = 1; > - } > - > if (pts != AV_NOPTS_VALUE) > pts += delay; > if (dts != AV_NOPTS_VALUE) > dts += delay; > } > > + if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { > + ts->first_pcr += dts * 300; > + ts->first_dts_checked = 1; > + } > + I think it's not same as the old code, the first_pcr will add extra delay if copyts is 0. > if (!ts_st->first_timestamp_checked && (pts == AV_NOPTS_VALUE || dts == AV_NOPTS_VALUE)) { > av_log(s, AV_LOG_ERROR, "first pts and dts value must be set\n"); > return AVERROR_INVALIDDATA; > -- > 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-request@ffmpeg.org with subject "unsubscribe".
On 02.11.2021 16:59, lance.lmwang@gmail.com wrote: [...] >> + if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { >> + ts->first_pcr += dts * 300; >> + ts->first_dts_checked = 1; >> + } >> + > > I think it's not same as the old code, the first_pcr will add extra delay if copyts is > 0. > proposed patch extend updating ts->first_pcr not only when copyts is 0 but when copyts is 1 (*-mpegts_copyts 1* specified) very similar issue i found at http://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242766.html
On Tue, Nov 02, 2021 at 05:36:37PM +0200, Maksym Veremeyenko wrote: > On 02.11.2021 16:59, lance.lmwang@gmail.com wrote: > [...] > > > + if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { > > > + ts->first_pcr += dts * 300; > > > + ts->first_dts_checked = 1; > > > + } > > > + > > > > I think it's not same as the old code, the first_pcr will add extra delay if copyts is > > 0. > > > > proposed patch extend updating ts->first_pcr not only when copyts is 0 but > when copyts is 1 (*-mpegts_copyts 1* specified) > > very similar issue i found at > http://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242766.html > I mean you need move the code before if (ts->copyts < 1) { instead of after. try with below command: ./ffmpeg -y -copyts -i http://samples.ffmpeg.org/MPEG2/foxksaz.ts -c:v libx264 -x264opts nal-hrd=cbr:force-cfr=1 \ -b:v 3500k -minrate 3500k -maxrate 3500k -bufsize 1000k -c:a mp2 -f mpegts \ -mpegts_copyts 0 -muxrate 4500k -vframes 1000 test.ts You'll get some "dts < pcr, TS is invalid" message. > -- > Maksym Veremeyenko > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 03.11.2021 11:19, lance.lmwang@gmail.com wrote: > On Tue, Nov 02, 2021 at 05:36:37PM +0200, Maksym Veremeyenko wrote: >> On 02.11.2021 16:59, lance.lmwang@gmail.com wrote: >> [...] >>>> + if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { >>>> + ts->first_pcr += dts * 300; >>>> + ts->first_dts_checked = 1; >>>> + } >>>> + >>> >>> I think it's not same as the old code, the first_pcr will add extra delay if copyts is >>> 0. [...] > > try with below command: > ./ffmpeg -y -copyts -i http://samples.ffmpeg.org/MPEG2/foxksaz.ts -c:v libx264 -x264opts nal-hrd=cbr:force-cfr=1 \ > -b:v 3500k -minrate 3500k -maxrate 3500k -bufsize 1000k -c:a mp2 -f mpegts \ > -mpegts_copyts 0 -muxrate 4500k -vframes 1000 test.ts > > You'll get some "dts < pcr, TS is invalid" message. > > yes, you are correct, in my tests s->max_delay was always 0 so i did not payed attention on order.
On 02.11.2021 12:47, Maksym Veremeyenko wrote: > One of latest commit > https://source.ffmpeg.org/?p=ffmpeg.git;a=commitdiff;h=6f36eb0da71d22aadf8f056f0966bd86656ea57e > claim it fixes endless loop on package generation if muxrate specified > and copyts used. But actually it does not work properly if > *-mpegts_copyts 1* specified: > > ffmpeg -y -copyts -i loewe.ts -c:v libx264 -x264opts > nal-hrd=cbr:force-cfr=1 -b:v 3500k -minrate 3500k -maxrate 3500k > -bufsize 1000k -c:a mp2 -f mpegts -mpegts_copyts 1 -muxrate 4500k > -vframes 1000 test.ts > > ffmpeg generate huge file until it reach zero-based pcr value equal to > first dts. > updated patch attached
On Wed, Nov 03, 2021 at 01:01:29PM +0200, Maksym Veremeyenko wrote: > On 02.11.2021 12:47, Maksym Veremeyenko wrote: > > One of latest commit https://source.ffmpeg.org/?p=ffmpeg.git;a=commitdiff;h=6f36eb0da71d22aadf8f056f0966bd86656ea57e > > claim it fixes endless loop on package generation if muxrate specified > > and copyts used. But actually it does not work properly if > > *-mpegts_copyts 1* specified: > > > > ffmpeg -y -copyts -i loewe.ts -c:v libx264 -x264opts > > nal-hrd=cbr:force-cfr=1 -b:v 3500k -minrate 3500k -maxrate 3500k > > -bufsize 1000k -c:a mp2 -f mpegts -mpegts_copyts 1 -muxrate 4500k > > -vframes 1000 test.ts > > > > ffmpeg generate huge file until it reach zero-based pcr value equal to > > first dts. > > > > updated patch attached > > -- > Maksym Veremeyenko > From 5d6265e84417d7169d9c225be7305c116c33042f Mon Sep 17 00:00:00 2001 > From: Maksym Veremeyenko <verem@m1.tv> > Date: Wed, 3 Nov 2021 12:57:45 +0200 > Subject: [PATCH] Fix first_pcr initial update > > --- > libavformat/mpegtsenc.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c > index 35c835c..dd01628 100644 > --- a/libavformat/mpegtsenc.c > +++ b/libavformat/mpegtsenc.c > @@ -1692,12 +1692,12 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt) > if (side_data) > stream_id = side_data[0]; > > - if (ts->copyts < 1) { > - if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { > - ts->first_pcr += dts * 300; > - ts->first_dts_checked = 1; > - } > + if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { > + ts->first_pcr += dts * 300; > + ts->first_dts_checked = 1; > + } > > + if (ts->copyts < 1) { > if (pts != AV_NOPTS_VALUE) > pts += delay; > if (dts != AV_NOPTS_VALUE) > -- > 1.8.3.1 > LGTM > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Wed, 3 Nov 2021, lance.lmwang@gmail.com wrote: > On Wed, Nov 03, 2021 at 01:01:29PM +0200, Maksym Veremeyenko wrote: >> On 02.11.2021 12:47, Maksym Veremeyenko wrote: >> > One of latest commit https://source.ffmpeg.org/?p=ffmpeg.git;a=commitdiff;h=6f36eb0da71d22aadf8f056f0966bd86656ea57e >> > claim it fixes endless loop on package generation if muxrate specified >> > and copyts used. But actually it does not work properly if >> > *-mpegts_copyts 1* specified: >> > >> > ffmpeg -y -copyts -i loewe.ts -c:v libx264 -x264opts >> > nal-hrd=cbr:force-cfr=1 -b:v 3500k -minrate 3500k -maxrate 3500k >> > -bufsize 1000k -c:a mp2 -f mpegts -mpegts_copyts 1 -muxrate 4500k >> > -vframes 1000 test.ts >> > >> > ffmpeg generate huge file until it reach zero-based pcr value equal to >> > first dts. >> > >> >> updated patch attached >> >> -- >> Maksym Veremeyenko > >> From 5d6265e84417d7169d9c225be7305c116c33042f Mon Sep 17 00:00:00 2001 >> From: Maksym Veremeyenko <verem@m1.tv> >> Date: Wed, 3 Nov 2021 12:57:45 +0200 >> Subject: [PATCH] Fix first_pcr initial update >> >> --- >> libavformat/mpegtsenc.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c >> index 35c835c..dd01628 100644 >> --- a/libavformat/mpegtsenc.c >> +++ b/libavformat/mpegtsenc.c >> @@ -1692,12 +1692,12 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt) >> if (side_data) >> stream_id = side_data[0]; >> >> - if (ts->copyts < 1) { >> - if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { >> - ts->first_pcr += dts * 300; >> - ts->first_dts_checked = 1; >> - } >> + if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { >> + ts->first_pcr += dts * 300; >> + ts->first_dts_checked = 1; >> + } >> >> + if (ts->copyts < 1) { >> if (pts != AV_NOPTS_VALUE) >> pts += delay; >> if (dts != AV_NOPTS_VALUE) >> -- >> 1.8.3.1 >> > > LGTM Thanks, will apply. Regards, Marton
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index 35c835c..32786be 100644 --- a/libavformat/mpegtsenc.c +++ b/libavformat/mpegtsenc.c @@ -1693,17 +1693,17 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt) stream_id = side_data[0]; if (ts->copyts < 1) { - if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { - ts->first_pcr += dts * 300; - ts->first_dts_checked = 1; - } - if (pts != AV_NOPTS_VALUE) pts += delay; if (dts != AV_NOPTS_VALUE) dts += delay; } + if (!ts->first_dts_checked && dts != AV_NOPTS_VALUE) { + ts->first_pcr += dts * 300; + ts->first_dts_checked = 1; + } + if (!ts_st->first_timestamp_checked && (pts == AV_NOPTS_VALUE || dts == AV_NOPTS_VALUE)) { av_log(s, AV_LOG_ERROR, "first pts and dts value must be set\n"); return AVERROR_INVALIDDATA;