[FFmpeg-devel] [PATCH v1] lavf/dashenc: Add option for calculting pkt duration

Jun Li junli1026 at gmail.com
Thu Apr 25 10:45:16 EEST 2019


On Wed, Apr 24, 2019 at 11:07 PM Jeyapal, Karthick <kjeyapal at akamai.com>
wrote:

>
> On 4/24/19 11:30 PM, Jun Li wrote:
> > On Sun, Apr 21, 2019 at 5:51 PM Jun Li <junli1026 at gmail.com> wrote:
> >
> >> Fix #7144.
> >> The current packet duration calculation is heuristic, which uses the
> >> historical durtion as current duration. This commit adds the option
> >> to buffer one packet and calcuate its duration when next packet arrives.
> Thanks for sending this patch. Here is opinion about this approach.
> Even when the next packet arrives you cannot calculate its duration
> reliably from pts or dts in muxer side.
> For example, when there are B-frames and variable frame rate this method
> of calculating duration from Next packet's DTS won't work.
> I believe that this issue should be fixed at the demuxer side before the
> raw stream passes thru an encoder.
> A raw stream's PTS from the capture device is guaranteed to be in-order
> and hence a demuxer can calculate its duration reliably from the next
> packet's PTS.
> Once it passes thru an encoder the frames could get out-of-order due to
> B-frames and using PTS on the muxer could give wrong duration for VFR.
> Basically, this method is also a heuristic method.
> Maybe it fixes the ticket you mentioned for that particular input(Maybe no
> B frames). But it won't work for all inputs.
> >> Obviously it adds one frame latency, which might be significant for
> >> VFR live content, so expose it as an option for user to choose.
> >> ---
> >>  doc/muxers.texi       |  4 ++++
> >>  libavformat/dashenc.c | 44 +++++++++++++++++++++++++++++++++++++++++--
> >>  2 files changed, 46 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/doc/muxers.texi b/doc/muxers.texi
> >> index ee99ef621e..76954877a6 100644
> >> --- a/doc/muxers.texi
> >> +++ b/doc/muxers.texi
> >> @@ -321,6 +321,10 @@ This is an experimental feature.
> >>  @item -master_m3u8_publish_rate @var{master_m3u8_publish_rate}
> >>  Publish master playlist repeatedly every after specified number of
> >> segment intervals.
> >>
> >> + at item -skip_estimate_pkt_duration
> >> +If this flag is set, packet duration will be calcuated as the diff of
> the
> >> current and next packet timestamp. The option is not for constant frame
> rate
> >> +content because heuristic estimation will be accurate in this case.
> >> Default value is 0.
> >> +
> >>  @end table
> >>
> >>  @anchor{framecrc}
> >> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> >> index 5f1333e436..f89d68a51b 100644
> >> --- a/libavformat/dashenc.c
> >> +++ b/libavformat/dashenc.c
> >> @@ -101,6 +101,7 @@ typedef struct OutputStream {
> >>      double availability_time_offset;
> >>      int total_pkt_size;
> >>      int muxer_overhead;
> >> +    AVPacket* prev_pkt;
> >>  } OutputStream;
> >>
> >>  typedef struct DASHContext {
> >> @@ -145,6 +146,7 @@ typedef struct DASHContext {
> >>      int ignore_io_errors;
> >>      int lhls;
> >>      int master_publish_rate;
> >> +    int skip_estiamte_pkt_duration;
> >>      int nr_of_streams_to_flush;
> >>      int nr_of_streams_flushed;
> >>  } DASHContext;
> >> @@ -1559,7 +1561,7 @@ static int dash_flush(AVFormatContext *s, int
> final,
> >> int stream)
> >>          } else {
> >>              snprintf(os->full_path, sizeof(os->full_path), "%s%s",
> >> c->dirname, os->initfile);
> >>          }
> >> -
> >> +
> >>          ret = flush_dynbuf(c, os, &range_length);
> >>          if (ret < 0)
> >>              break;
> >> @@ -1643,7 +1645,7 @@ static int dash_flush(AVFormatContext *s, int
> final,
> >> int stream)
> >>      return ret;
> >>  }
> >>
> >> -static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)
> >> +static int dash_write_packet_internal(AVFormatContext *s, AVPacket
> *pkt)
> >>  {
> >>      DASHContext *c = s->priv_data;
> >>      AVStream *st = s->streams[pkt->stream_index];
> >> @@ -1789,11 +1791,47 @@ static int dash_write_packet(AVFormatContext *s,
> >> AVPacket *pkt)
> >>      return ret;
> >>  }
> >>
> >> +static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)
> >> +{
> >> +    DASHContext *c = s->priv_data;
> >> +    if (!c->skip_estiamte_pkt_duration)
> >> +        return dash_write_packet_internal(s, pkt);
> >> +
> >> +    AVStream *st = s->streams[pkt->stream_index];
> >> +    OutputStream *os = &c->streams[pkt->stream_index];
> >> +    int ret = 0;
> >> +
> >> +    if (os->prev_pkt) {
> >> +        if (!os->prev_pkt->duration && pkt->dts >= os->prev_pkt->dts)
> >> +            os->prev_pkt->duration = pkt->dts - os->prev_pkt->dts;
> >> +        ret = dash_write_packet_internal(s, os->prev_pkt);
> >> +        av_packet_unref(os->prev_pkt);
> >> +        if (av_packet_ref(os->prev_pkt, pkt)) {
> >> +            av_log(s, AV_LOG_ERROR, "Failed to set up a reference to
> >> current packet, lost one packet for muxing.\n");
> >> +            av_packet_free(&os->prev_pkt);
> >> +        }
> >> +    } else {
> >> +        os->prev_pkt = av_packet_clone(pkt);
> >> +    }
> >> +    return ret;
> >> +}
> >> +
> >>  static int dash_write_trailer(AVFormatContext *s)
> >>  {
> >>      DASHContext *c = s->priv_data;
> >>      int i;
> >>
> >> +    if (c->skip_estiamte_pkt_duration) {
> >> +        for (i = 0; i < s->nb_streams; i++) {
> >> +            OutputStream *os = &c->streams[i];
> >> +            if (os->prev_pkt) {
> >> +                // write last packet
> >> +                dash_write_packet_internal(s, os->prev_pkt);
> >> +                av_packet_free(&os->prev_pkt);
> >> +            }
> >> +        }
> >> +    }
> >> +
> >>      if (s->nb_streams > 0) {
> >>          OutputStream *os = &c->streams[0];
> >>          // If no segments have been written so far, try to do a crude
> >> @@ -1888,6 +1926,8 @@ static const AVOption options[] = {
> >>      { "ignore_io_errors", "Ignore IO errors during open and write.
> Useful
> >> for long-duration runs with network output", OFFSET(ignore_io_errors),
> >> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, E },
> >>      { "lhls", "Enable Low-latency HLS(Experimental). Adds
> #EXT-X-PREFETCH
> >> tag with current segment's URI", OFFSET(lhls), AV_OPT_TYPE_BOOL, { .i64
> = 0
> >> }, 0, 1, E },
> >>      { "master_m3u8_publish_rate", "Publish master playlist every after
> >> this many segment intervals", OFFSET(master_publish_rate),
> AV_OPT_TYPE_INT,
> >> {.i64 = 0}, 0, UINT_MAX, E},
> >> +    { "skip_estimate_pkt_duration", "Skip estimate packet duration,
> >> buffer previous packet for calculating its duration.",
> >> OFFSET(skip_estiamte_pkt_duration), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0,
> 1, E
> >> },
> >> +
> >>      { NULL },
> >>  };
> >>
> >> --
> >> 2.17.1
> >
> >
> > Ping.
> > _______________________________________________
> > 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 for review Karthick !
I agree with you that this patch does not apply the case "*B frame + VFR*",
but I think it woks for "*No B frame + VFR*".
While the current implementation does not work for VFR no matter
with/without B frame, is that right ? What do you think ?

Best Regards,
Jun


More information about the ffmpeg-devel mailing list