On Fri, 27 Jan 2017, Peter Große wrote:
The current implementation creates new segments comparing
pkt->pts - first_pts > total_duration
I guess this would be more understandable written like this:
pkt->pts - first_pts > nb_segs * min_seg_duration
or
total_duration > nb_segs * min_seg_duration
This works fine, but if the keyframe interval is smaller than "min_seg_duration"
segments shorter than the minimum segment duration are created.
Example: keyint=50, min_seg_duration=3000000
segment 1 contains keyframe 1 (duration=2s < total_duration=3s)
and keyframe 2 (duration=4s >= total_duration=3s)
segment 2 contains keyframe 3 (duration=6s >= total_duration=6s)
segment 3 contains keyframe 4 (duration=8s < total_duration=9s)
and keyframe 5 (duration=10s >= total_duration=9s)
...
Segment 2 is only 2s long, shorter than min_seg_duration = 3s.
FWIW, this was the indended behaviour originally, but I agree that the
option name isn't quite optimal given this.
To fix this, new segments are created based on the actual written duration.
Otherwise the option name "min_seg_duration" is misleading.
Signed-off-by: Peter Große <[email protected]>
---
libavformat/dashenc.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 198932c..93c292f 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -62,7 +62,7 @@ typedef struct OutputStream {
int ctx_inited;
uint8_t iobuf[32768];
AVIOContext *out;
- int packets_written;
+ int duration_written;
char initfile[1024];
int64_t init_start_pos;
int init_range_length;
@@ -785,7 +785,7 @@ static int dash_flush(AVFormatContext *s, int final, int
stream)
int64_t start_pos;
int range_length, index_length = 0;
- if (!os->packets_written)
+ if (!os->duration_written)
continue;
// Flush the single stream that got a keyframe right now.
@@ -823,7 +823,7 @@ static int dash_flush(AVFormatContext *s, int final, int
stream)
av_write_frame(os->ctx, NULL);
avio_flush(os->ctx->pb);
- os->packets_written = 0;
+ os->duration_written = 0;
range_length = avio_tell(os->ctx->pb) - start_pos;
if (c->single_file) {
@@ -868,7 +868,6 @@ static int dash_write_packet(AVFormatContext *s, AVPacket
*pkt)
DASHContext *c = s->priv_data;
AVStream *st = s->streams[pkt->stream_index];
OutputStream *os = &c->streams[pkt->stream_index];
- int64_t seg_end_duration = (os->segment_index) * (int64_t)
c->min_seg_duration;
int ret;
ret = update_stream_extradata(s, os, st->codecpar);
@@ -897,9 +896,9 @@ static int dash_write_packet(AVFormatContext *s, AVPacket
*pkt)
os->first_pts = pkt->pts;
if ((!c->has_video || st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) &&
- pkt->flags & AV_PKT_FLAG_KEY && os->packets_written &&
- av_compare_ts(pkt->pts - os->first_pts, st->time_base,
- seg_end_duration, AV_TIME_BASE_Q) >= 0) {
+ pkt->flags & AV_PKT_FLAG_KEY && os->duration_written &&
+ av_compare_ts(os->duration_written + pkt->duration, st->time_base,
+ c->min_seg_duration, AV_TIME_BASE_Q) >= 0) {
int64_t prev_duration = c->last_duration;
c->last_duration = av_rescale_q(pkt->pts - os->start_pts,
@@ -922,7 +921,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket
*pkt)
return ret;
}
- if (!os->packets_written) {
+ if (!os->duration_written) {
// If we wrote a previous segment, adjust the start time of the segment
// to the end of the previous one (which is the same as the mp4 muxer
// does). This avoids gaps in the timeline.
@@ -935,7 +934,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket
*pkt)
os->max_pts = pkt->pts + pkt->duration;
else
os->max_pts = FFMAX(os->max_pts, pkt->pts + pkt->duration);
- os->packets_written++;
+ os->duration_written += pkt->duration;
return ff_write_chained(os->ctx, 0, pkt, s);
}
This isn't the best way of achieving what you're trying to do.
Don't accumulate pkt->duration values. In general we can't really assume
that pkt->duration is set sensibly.
In some places we need an estimate of the duration of a packet, and in
those cases we fill it in using the duration of the previous packet (i.e.
the diff between this packets timestamp and the previous one) if we don't
have a definitive value available.
In those cases where we use an estimate of the duration, we make sure to
correct it as soon as possible when we get the real timestamps of the next
packet. But we shouldn't accumulate the values since that would end up
with accumulating the error, blowing it out of propertion at worst.
To get the behaviour you want, couldn't you just change the comparison
like this:
- av_compare_ts(pkt->pts - os->first_pts, st->time_base,
- seg_end_duration, AV_TIME_BASE_Q) >= 0) {
+ av_compare_ts(pkt->pts - os->start_pts, st->time_base,
+ c->min_seg_duration, AV_TIME_BASE_Q) >= 0) {
We already have the start timestamp of the current segment in
os->start_pts.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
