diff mbox

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

Message ID 20190422005124.15109-1-junli1026@gmail.com
State New
Headers show

Commit Message

Jun Li April 22, 2019, 12:51 a.m. UTC
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.
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(-)

Comments

Jun Li April 24, 2019, 6 p.m. UTC | #1
On Sun, Apr 21, 2019 at 5:51 PM Jun Li <junli1026@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.
> 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.
>
> +@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.
Jeyapal, Karthick April 25, 2019, 6:07 a.m. UTC | #2
On 4/24/19 11:30 PM, Jun Li wrote:
> On Sun, Apr 21, 2019 at 5:51 PM Jun Li <junli1026@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.

>>

>> +@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@ffmpeg.org

> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

>

> To unsubscribe, visit link above, or email

> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Jun Li April 25, 2019, 7:45 a.m. UTC | #3
On Wed, Apr 24, 2019 at 11:07 PM Jeyapal, Karthick <kjeyapal@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@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.
> >>
> >> +@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@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@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
Jeyapal, Karthick April 25, 2019, 8:28 a.m. UTC | #4
On 4/25/19 1:15 PM, Jun Li wrote:
>

>

>

> On Wed, Apr 24, 2019 at 11:07 PM Jeyapal, Karthick <kjeyapal@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@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.

>     >>

>     >> +@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@ffmpeg.org

>     > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

>     >

>     > To unsubscribe, visit link above, or email

>     > ffmpeg-devel-request@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

>

>

>

>
diff mbox

Patch

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.
 
+@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 },
 };