On 4/25/19 1:15 PM, Jun Li wrote:
>
>
>
> On Wed, Apr 24, 2019 at 11:07 PM Jeyapal, Karthick <kjeya...@akamai.com>
> wrote:
>
>
> On 4/24/19 11:30 PM, Jun Li wrote:
> > On Sun, Apr 21, 2019 at 5:51 PM Jun Li <junli1...@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-requ...@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
>
>
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
