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

Jeyapal, Karthick kjeyapal at akamai.com
Thu Apr 25 11:28:15 EEST 2019


On 4/25/19 1:15 PM, Jun Li wrote:
>
>
>
> 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 ? 
I think dashenc muxer is not the right place to fix this bug. I think this should be fixed in the demuxer source that supports VFR. Such a demuxer should set the packet duration correctly.
>
> Best Regards,
> Jun
>
>
>
>



More information about the ffmpeg-devel mailing list