| Message ID | 20171010173658.13922-1-isasi@google.com |
|---|---|
| State | Accepted |
| Commit | 2b006ccf8318d84101ed83b75df4c9682a963217 |
| Headers | show |
On Tue, 10 Oct 2017 10:36:58 -0700 Sasi Inguva <isasi-at-google.com@ffmpeg.org> wrote: > This is required for FLV files, for which duration_pts comes out to be zero. > > Signed-off-by: Sasi Inguva <isasi@google.com> > --- > fftools/ffmpeg.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 6d64bc1043..3ee31473dc 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo > ist->next_dts = AV_NOPTS_VALUE; > } > > - if (got_output) > - ist->next_pts += av_rescale_q(duration_pts, ist->st->time_base, AV_TIME_BASE_Q); > + if (got_output) { > + if (duration_pts > 0) { > + ist->next_pts += av_rescale_q(duration_pts, ist->st->time_base, AV_TIME_BASE_Q); > + } else { > + ist->next_pts += duration_dts; > + } > + } > break; > case AVMEDIA_TYPE_SUBTITLE: > if (repeating) So why can't duration_pts be 0?
2017-10-10 19:44 GMT+02:00 wm4 <nfxjfg@googlemail.com>: > On Tue, 10 Oct 2017 10:36:58 -0700 > Sasi Inguva <isasi-at-google.com@ffmpeg.org> wrote: > > > This is required for FLV files, for which duration_pts comes out to be > zero. > > > > Signed-off-by: Sasi Inguva <isasi@google.com> > > --- > > fftools/ffmpeg.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index 6d64bc1043..3ee31473dc 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream > *ist, const AVPacket *pkt, int no_eo > > ist->next_dts = AV_NOPTS_VALUE; > > } > > > > - if (got_output) > > - ist->next_pts += av_rescale_q(duration_pts, > ist->st->time_base, AV_TIME_BASE_Q); > > + if (got_output) { > > + if (duration_pts > 0) { > > + ist->next_pts += av_rescale_q(duration_pts, > ist->st->time_base, AV_TIME_BASE_Q); > > + } else { > > + ist->next_pts += duration_dts; > > + } > > + } > > break; > > case AVMEDIA_TYPE_SUBTITLE: > > if (repeating) > > So why can't duration_pts be 0? > Hmm, to my understanding that would be a frame with no duration. Virtually an invisible frame. Can you give an example?
2017-10-10 19:36 GMT+02:00 Sasi Inguva <isasi-at-google.com@ffmpeg.org>: > This is required for FLV files, for which duration_pts comes out to be > zero. > > Signed-off-by: Sasi Inguva <isasi@google.com> > --- > fftools/ffmpeg.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 6d64bc1043..3ee31473dc 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream *ist, > const AVPacket *pkt, int no_eo > ist->next_dts = AV_NOPTS_VALUE; > } > > - if (got_output) > - ist->next_pts += av_rescale_q(duration_pts, > ist->st->time_base, AV_TIME_BASE_Q); > + if (got_output) { > + if (duration_pts > 0) { > + ist->next_pts += av_rescale_q(duration_pts, > ist->st->time_base, AV_TIME_BASE_Q); > + } else { > + ist->next_pts += duration_dts; > + } > + } > break; > case AVMEDIA_TYPE_SUBTITLE: > if (repeating) > -- > Patch LGTM. Since before commit 36c111c40f4bd7da114df0e9c484833aa2cdf2dc duration_dts was generally used here, it should be ok as fallback.
On Tue, Oct 10, 2017 at 10:26:13PM +0200, Thomas Mundt wrote: > 2017-10-10 19:36 GMT+02:00 Sasi Inguva <isasi-at-google.com@ffmpeg.org>: > > > This is required for FLV files, for which duration_pts comes out to be > > zero. > > > > Signed-off-by: Sasi Inguva <isasi@google.com> > > --- > > fftools/ffmpeg.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index 6d64bc1043..3ee31473dc 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream *ist, > > const AVPacket *pkt, int no_eo > > ist->next_dts = AV_NOPTS_VALUE; > > } > > > > - if (got_output) > > - ist->next_pts += av_rescale_q(duration_pts, > > ist->st->time_base, AV_TIME_BASE_Q); > > + if (got_output) { > > + if (duration_pts > 0) { > > + ist->next_pts += av_rescale_q(duration_pts, > > ist->st->time_base, AV_TIME_BASE_Q); > > + } else { > > + ist->next_pts += duration_dts; > > + } > > + } > > break; > > case AVMEDIA_TYPE_SUBTITLE: > > if (repeating) > > -- > > > > Patch LGTM. will apply thx > Since before commit 36c111c40f4bd7da114df0e9c484833aa2cdf2dc duration_dts > was generally used here, it should be ok as fallback. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Hi Michael, 2017-10-12 1:28 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>: > On Tue, Oct 10, 2017 at 10:26:13PM +0200, Thomas Mundt wrote: > > 2017-10-10 19:36 GMT+02:00 Sasi Inguva <isasi-at-google.com@ffmpeg.org>: > > > > > This is required for FLV files, for which duration_pts comes out to be > > > zero. > > > > > > Signed-off-by: Sasi Inguva <isasi@google.com> > > > --- > > > fftools/ffmpeg.c | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > > index 6d64bc1043..3ee31473dc 100644 > > > --- a/fftools/ffmpeg.c > > > +++ b/fftools/ffmpeg.c > > > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream > *ist, > > > const AVPacket *pkt, int no_eo > > > ist->next_dts = AV_NOPTS_VALUE; > > > } > > > > > > - if (got_output) > > > - ist->next_pts += av_rescale_q(duration_pts, > > > ist->st->time_base, AV_TIME_BASE_Q); > > > + if (got_output) { > > > + if (duration_pts > 0) { > > > + ist->next_pts += av_rescale_q(duration_pts, > > > ist->st->time_base, AV_TIME_BASE_Q); > > > + } else { > > > + ist->next_pts += duration_dts; > > > + } > > > + } > > > break; > > > case AVMEDIA_TYPE_SUBTITLE: > > > if (repeating) > > > -- > > > > > > > Patch LGTM. > > will apply > would you mind pushing this patch to the 3.4 branch also? Thank you, Thomas
On Thu, 12 Oct 2017 01:28:01 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Oct 10, 2017 at 10:26:13PM +0200, Thomas Mundt wrote: > > 2017-10-10 19:36 GMT+02:00 Sasi Inguva <isasi-at-google.com@ffmpeg.org>: > > > > > This is required for FLV files, for which duration_pts comes out to be > > > zero. > > > > > > Signed-off-by: Sasi Inguva <isasi@google.com> > > > --- > > > fftools/ffmpeg.c | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > > index 6d64bc1043..3ee31473dc 100644 > > > --- a/fftools/ffmpeg.c > > > +++ b/fftools/ffmpeg.c > > > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream *ist, > > > const AVPacket *pkt, int no_eo > > > ist->next_dts = AV_NOPTS_VALUE; > > > } > > > > > > - if (got_output) > > > - ist->next_pts += av_rescale_q(duration_pts, > > > ist->st->time_base, AV_TIME_BASE_Q); > > > + if (got_output) { > > > + if (duration_pts > 0) { > > > + ist->next_pts += av_rescale_q(duration_pts, > > > ist->st->time_base, AV_TIME_BASE_Q); > > > + } else { > > > + ist->next_pts += duration_dts; > > > + } > > > + } > > > break; > > > case AVMEDIA_TYPE_SUBTITLE: > > > if (repeating) > > > -- > > > > > > > Patch LGTM. > > will apply I don't feel like this was sufficiently discussed.
I can initialize duration_pts to -1, and check against duration_pts < 0 , if you think that's better. On Thu, Oct 12, 2017 at 3:51 AM, wm4 <nfxjfg@googlemail.com> wrote: > On Thu, 12 Oct 2017 01:28:01 +0200 > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > On Tue, Oct 10, 2017 at 10:26:13PM +0200, Thomas Mundt wrote: > > > 2017-10-10 19:36 GMT+02:00 Sasi Inguva <isasi-at-google.com@ffmpeg.org > >: > > > > > > > This is required for FLV files, for which duration_pts comes out to > be > > > > zero. > > > > > > > > Signed-off-by: Sasi Inguva <isasi@google.com> > > > > --- > > > > fftools/ffmpeg.c | 9 +++++++-- > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > > > index 6d64bc1043..3ee31473dc 100644 > > > > --- a/fftools/ffmpeg.c > > > > +++ b/fftools/ffmpeg.c > > > > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream > *ist, > > > > const AVPacket *pkt, int no_eo > > > > ist->next_dts = AV_NOPTS_VALUE; > > > > } > > > > > > > > - if (got_output) > > > > - ist->next_pts += av_rescale_q(duration_pts, > > > > ist->st->time_base, AV_TIME_BASE_Q); > > > > + if (got_output) { > > > > + if (duration_pts > 0) { > > > > + ist->next_pts += av_rescale_q(duration_pts, > > > > ist->st->time_base, AV_TIME_BASE_Q); > > > > + } else { > > > > + ist->next_pts += duration_dts; > > > > + } > > > > + } > > > > break; > > > > case AVMEDIA_TYPE_SUBTITLE: > > > > if (repeating) > > > > -- > > > > > > > > > > Patch LGTM. > > > > will apply > > I don't feel like this was sufficiently discussed. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Thu, Oct 12, 2017 at 10:43:54AM +0200, Thomas Mundt wrote: > Hi Michael, > > 2017-10-12 1:28 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>: > > > On Tue, Oct 10, 2017 at 10:26:13PM +0200, Thomas Mundt wrote: > > > 2017-10-10 19:36 GMT+02:00 Sasi Inguva <isasi-at-google.com@ffmpeg.org>: > > > > > > > This is required for FLV files, for which duration_pts comes out to be > > > > zero. > > > > > > > > Signed-off-by: Sasi Inguva <isasi@google.com> > > > > --- > > > > fftools/ffmpeg.c | 9 +++++++-- > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > > > index 6d64bc1043..3ee31473dc 100644 > > > > --- a/fftools/ffmpeg.c > > > > +++ b/fftools/ffmpeg.c > > > > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream > > *ist, > > > > const AVPacket *pkt, int no_eo > > > > ist->next_dts = AV_NOPTS_VALUE; > > > > } > > > > > > > > - if (got_output) > > > > - ist->next_pts += av_rescale_q(duration_pts, > > > > ist->st->time_base, AV_TIME_BASE_Q); > > > > + if (got_output) { > > > > + if (duration_pts > 0) { > > > > + ist->next_pts += av_rescale_q(duration_pts, > > > > ist->st->time_base, AV_TIME_BASE_Q); > > > > + } else { > > > > + ist->next_pts += duration_dts; > > > > + } > > > > + } > > > > break; > > > > case AVMEDIA_TYPE_SUBTITLE: > > > > if (repeating) > > > > -- > > > > > > > > > > Patch LGTM. > > > > will apply > > > > would you mind pushing this patch to the 3.4 branch also? ok, will be part of by next push to release/3.4 unless someone wants this to be omitted [...]
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 6d64bc1043..3ee31473dc 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo ist->next_dts = AV_NOPTS_VALUE; } - if (got_output) - ist->next_pts += av_rescale_q(duration_pts, ist->st->time_base, AV_TIME_BASE_Q); + if (got_output) { + if (duration_pts > 0) { + ist->next_pts += av_rescale_q(duration_pts, ist->st->time_base, AV_TIME_BASE_Q); + } else { + ist->next_pts += duration_dts; + } + } break; case AVMEDIA_TYPE_SUBTITLE: if (repeating)
This is required for FLV files, for which duration_pts comes out to be zero. Signed-off-by: Sasi Inguva <isasi@google.com> --- fftools/ffmpeg.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)