[FFmpeg-devel] [mov] Fix trampling of ctts during seeks when sidx support is enabled.
Dale Curtis
dalecurtis at chromium.org
Tue Jul 18 21:49:26 EEST 2017
Updated patch that fixes other ctts modification code to use the new
ctts_allocated_size value; I've verified this passes fate.
- dale
On Tue, Jul 18, 2017 at 9:53 AM, Dale Curtis <dalecurtis at chromium.org>
wrote:
> Resending as it's own message per contribution rules. I've also attached
> the patch in case the formatting gets trashed.
>
> When sidx box support is enabled, the code will skip reading all
> trun boxes (each containing ctts entries for samples inthat box).
>
> If seeks are attempted before all ctts values are known, the old
> code would dump ctts entries into the wrong location. These are
> then used to compute pts values which leads to out of order and
> incorrectly timestamped packets.
>
> This patch fixes ctts processing by always using the index returned
> by av_add_index_entry() as the ctts_data index. When the index gains
> new entries old values are reshuffled as appropriate.
>
> This approach makes sense since the mov demuxer is already relying
> on the mapping of AVIndex entries to samples for correct demuxing.
>
> Notes for future improvement:
> Probably there are other boxes (stts, stsc, etc) that are impacted
> by this issue... this patch only attempts to fix ctts since it
> completely breaks packet timestamping.
>
> This patch continues using an array for the ctts data, which is not
> the most ideal given the rearrangement that needs to happen (via
> memmove as new entries are read in). Ideally AVIndex and the ctts
> data would be set-type structures so addition is always worst case
> O(lg(n)) instead of the O(n^2) that exists now; this slowdown is
> noticeable during seeks.
>
> Additionally since ctts samples from trun boxes always have a count
> of 1, there's probably an opportunity to avoid the post-seek fixup
> that iterates O(n) over all samples with an O(1) when no non-1 count
> samples are present.
>
> Signed-off-by: Dale Curtis <dalecurtis at chromium.org>
> ---
> libavformat/isom.h | 1 +
> libavformat/mov.c | 46 +++++++++++++++++++++++++++++++---------------
> 2 files changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index ff009b0896..fdd98c28f5 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -137,6 +137,7 @@ typedef struct MOVStreamContext {
> unsigned int stts_count;
> MOVStts *stts_data;
> unsigned int ctts_count;
> + unsigned int ctts_allocated_size;
> MOVStts *ctts_data;
> unsigned int stsc_count;
> MOVStsc *stsc_data;
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 63f84be782..412290b435 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4297,11 +4297,6 @@ static int mov_read_trun(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> }
> if ((uint64_t)entries+sc->ctts_count >=
> UINT_MAX/sizeof(*sc->ctts_data))
> return AVERROR_INVALIDDATA;
> - if ((err = av_reallocp_array(&sc->ctts_data, entries +
> sc->ctts_count,
> - sizeof(*sc->ctts_data))) < 0) {
> - sc->ctts_count = 0;
> - return err;
> - }
> if (flags & MOV_TRUN_DATA_OFFSET) data_offset =
> avio_rb32(pb);
> if (flags & MOV_TRUN_FIRST_SAMPLE_FLAGS) first_sample_flags =
> avio_rb32(pb);
> dts = sc->track_end - sc->time_offset;
> @@ -4312,26 +4307,28 @@ static int mov_read_trun(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
> unsigned sample_size = frag->size;
> int sample_flags = i ? frag->flags : first_sample_flags;
> unsigned sample_duration = frag->duration;
> + unsigned ctts_duration = 0;
> int keyframe = 0;
> + int ctts_index = 0;
> + int old_nb_index_entries = st->nb_index_entries;
>
> if (flags & MOV_TRUN_SAMPLE_DURATION) sample_duration =
> avio_rb32(pb);
> if (flags & MOV_TRUN_SAMPLE_SIZE) sample_size =
> avio_rb32(pb);
> if (flags & MOV_TRUN_SAMPLE_FLAGS) sample_flags =
> avio_rb32(pb);
> - sc->ctts_data[sc->ctts_count].count = 1;
> - sc->ctts_data[sc->ctts_count].duration = (flags &
> MOV_TRUN_SAMPLE_CTS) ?
> - avio_rb32(pb) : 0;
> - mov_update_dts_shift(sc, sc->ctts_data[sc->ctts_count].duration);
> + if (flags & MOV_TRUN_SAMPLE_CTS) ctts_duration =
> avio_rb32(pb);
> +
> + mov_update_dts_shift(sc, ctts_duration);
> if (frag->time != AV_NOPTS_VALUE) {
> if (c->use_mfra_for == FF_MOV_FLAG_MFRA_PTS) {
> int64_t pts = frag->time;
> av_log(c->fc, AV_LOG_DEBUG, "found frag time %"PRId64
> " sc->dts_shift %d ctts.duration %d"
> " sc->time_offset %"PRId64" flags &
> MOV_TRUN_SAMPLE_CTS %d\n", pts,
> - sc->dts_shift, sc->ctts_data[sc->ctts_count].
> duration,
> + sc->dts_shift, ctts_duration,
> sc->time_offset, flags & MOV_TRUN_SAMPLE_CTS);
> dts = pts - sc->dts_shift;
> if (flags & MOV_TRUN_SAMPLE_CTS) {
> - dts -= sc->ctts_data[sc->ctts_count].duration;
> + dts -= ctts_duration;
> } else {
> dts -= sc->time_offset;
> }
> @@ -4343,7 +4340,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> }
> frag->time = AV_NOPTS_VALUE;
> }
> - sc->ctts_count++;
> +
> if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
> keyframe = 1;
> else
> @@ -4352,11 +4349,30 @@ static int mov_read_trun(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
> MOV_FRAG_SAMPLE_FLAG_DEPENDS_YES));
> if (keyframe)
> distance = 0;
> - err = av_add_index_entry(st, offset, dts, sample_size, distance,
> - keyframe ? AVINDEX_KEYFRAME : 0);
> - if (err < 0) {
> + ctts_index = av_add_index_entry(st, offset, dts, sample_size,
> distance,
> + keyframe ? AVINDEX_KEYFRAME : 0);
> + if (ctts_index >= 0 && old_nb_index_entries <
> st->nb_index_entries) {
> + unsigned int size_needed = st->nb_index_entries *
> sizeof(*sc->ctts_data);
> + unsigned int request_size = size_needed >
> sc->ctts_allocated_size ?
> + FFMAX(size_needed, 2 * sc->ctts_allocated_size) :
> size_needed;
> + sc->ctts_data = av_fast_realloc(sc->ctts_data,
> &sc->ctts_allocated_size, request_size);
> + if (!sc->ctts_data) {
> + sc->ctts_count = 0;
> + return AVERROR(ENOMEM);
> + }
> +
> + if (ctts_index != old_nb_index_entries) {
> + memmove(sc->ctts_data + ctts_index + 1, sc->ctts_data +
> ctts_index,
> + sizeof(*sc->ctts_data) * (sc->ctts_count -
> ctts_index));
> + }
> +
> + sc->ctts_data[ctts_index].count = 1;
> + sc->ctts_data[ctts_index].duration = ctts_duration;
> + sc->ctts_count++;
> + } else {
> av_log(c->fc, AV_LOG_ERROR, "Failed to add index entry\n");
> }
> +
> av_log(c->fc, AV_LOG_TRACE, "AVIndex stream %d, sample %u, offset
> %"PRIx64", dts %"PRId64", "
> "size %u, distance %d, keyframe %d\n", st->index,
> sc->sample_count+i,
> offset, dts, sample_size, distance, keyframe);
> --
> 2.13.2.932.g7449e964c-goog
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-trampling-of-ctts-during-seeks-when-sidx-support.patch
Type: text/x-patch
Size: 10429 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170718/0fc64e12/attachment.bin>
More information about the ffmpeg-devel
mailing list