From patchwork Tue Jul 18 16:53:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dale Curtis X-Patchwork-Id: 4357 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.1.76 with SMTP id 73csp1814938vsb; Tue, 18 Jul 2017 09:53:33 -0700 (PDT) X-Received: by 10.28.125.74 with SMTP id y71mr2591238wmc.46.1500396813321; Tue, 18 Jul 2017 09:53:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1500396813; cv=none; d=google.com; s=arc-20160816; b=fhQtzZZwdKNFVMVa/zFL5TtQu73f1nF6VXY77ktS0LQ7oKMqYk2Mc7Up/aNlq2/HoT N91JEmqJPRmxRpm618dyAoe3CuB1UQzAIrsT+MYhIkwPit3BAQ3ZMXjV6q7o/7domln9 iL6mhN6HyHFbDwz5z+c/UhA9Q7lQjKSuSusnJ5LTSKUo6bNfEwe3iMiG5SIhZQEJCgbT +kUJg7NUFfY4xcnXme48DWIz2hN0+eA0Q+ouz9Az8DiV9T1o/J46V9QQxlwcaiXxnRZL +kCwlja/UhQbCaaFeEXgPVhDcmG8NRj+d+1CH6Pud2DoFuXP9qNrI8Vu0trReZiQ1eyg YaEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:mime-version:dkim-signature:dkim-signature :delivered-to:arc-authentication-results; bh=OodyWEcl8kPT2vcUOJk5p8WKD5hhjcgy443g0cSZY+k=; b=fCulo3O1pmZNWVkJqFD9QxIKQqrK5s7psGkfUjYEx6D0GrY7XOiCDjQRw0d3QHm/rS YLxS+xph6VZ6LCfi7KZwA7ne/hq+LxjouBEBIoo+uubcoEw5AuiwrvVZoBLa3XInTPcm 1zAC7Y1Me/fti9oWmrs9EVREsIDIKRgCQIadir/hX6vx2sKWgYsGesqRAAroIH9lr7Kb 5RJDSZWh+LsHs3umby/yIBceoU3ufiOSZ/CBOIdn4U5pF9rAHsO9GIlnMMBLggr1IMPC 79TviBxKOnyFLsmLfh4J58O60Sn9mQg8OEvgYk/mAMM9n5wQRRRt9+95RDRFxlfs5ipe IQkw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.b=MQ1w7cRM; dkim=neutral (body hash did not verify) header.i=@chromium.org header.b=GrLfAq6A; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id c187si2676250wmd.146.2017.07.18.09.53.32; Tue, 18 Jul 2017 09:53:33 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.b=MQ1w7cRM; dkim=neutral (body hash did not verify) header.i=@chromium.org header.b=GrLfAq6A; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 02229689934; Tue, 18 Jul 2017 19:53:23 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr0-f179.google.com (mail-wr0-f179.google.com [209.85.128.179]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 478CB68971B for ; Tue, 18 Jul 2017 19:53:16 +0300 (EEST) Received: by mail-wr0-f179.google.com with SMTP id 12so37567330wrb.1 for ; Tue, 18 Jul 2017 09:53:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:sender:from:date:message-id:subject:to; bh=myehdhCy2XDoVQBCZcWYurEJLhwV46eRZPqkM1PfOhQ=; b=MQ1w7cRMq8bvdnd36Aiwo8O3Z5JrhNG71zg4fY++fkGOXYBMDwsV0oiVUw0i+KjSQr tJeAnrGJ3rEYf2wKDXRiZ/BkvkaaZElU4aJ6bN34eh2cSrrKNkHaO2AxATL9+LJ91qfh aAMjwtTSE/MEKasn+Gl4qbG5K41DyNfKHXsowGwDjmmSXiYNAs83viH/9+TfhRTOqi5d 8kHfEhLdo5crZ5pHlOvi3TF8Lak1ElnVE7uxIps13Q4JAZhcLAlb/p0Gwuga9idJYTUx iqlI8azniX2lT1SH4bKvn3R1XzjI5239nyL0xhH5ohcoNsvNcWdY0cMdGAdJLhhXwM4+ NO7A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:sender:from:date:message-id:subject:to; bh=myehdhCy2XDoVQBCZcWYurEJLhwV46eRZPqkM1PfOhQ=; b=GrLfAq6AlWX0Y3c8Ynwtf98Dm7n/rHcRhM2r/QPuvu/yostyonbjVbxupzex2bW2SA LDLRDebb0rqojp68aQgFRhwcs1Wm0M6qfhu509vlJo55hlW7rfERPur1uotxYrR6iBD4 buUhLOQilq50/svoNfH7k7qBaPBnw+QOiGEGQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:from:date:message-id:subject :to; bh=myehdhCy2XDoVQBCZcWYurEJLhwV46eRZPqkM1PfOhQ=; b=fAFN775uqxAXpXakGzH6JLPwcTDwTWWfT42fHjuKPevOHmZ0J22fqIAxsg6BVvIcoV 0saFIjtMBpjMO8ldVN3B9l4Oejlh0WDwXcMbPCnu/J/PN25u5Ofdj791nrCWJH+QMi1X MIBaXoXlkQdBkwqaKjDfioZFME4+BArnEZoU5HsAhIERmWnNrxZL9ksMgaxs5UdUOJ1I BWUv22EXMYzkfFSTO5eqkLto4TtR6zt01hikbnFg5849BXHAHAY9qEiRhpXf+h6/B49X lYQIn3BVA11fWU5jURY6QTQwjF443ivV9dHo/OTnQ0uBtjjXws72f6/urJOt0UX3agEU 8o9g== X-Gm-Message-State: AIVw113SCbNh7jifRnKGXFoOQ23FqmfzwTPI+2/N00yLQKzUhoar1+vy 0AcYxxLCKtZ0DQOhSLzL3hEWLTI6x97+Fw0= X-Received: by 10.223.133.35 with SMTP id 32mr1783394wrh.30.1500396802463; Tue, 18 Jul 2017 09:53:22 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.71.149 with HTTP; Tue, 18 Jul 2017 09:53:01 -0700 (PDT) From: Dale Curtis Date: Tue, 18 Jul 2017 09:53:01 -0700 X-Google-Sender-Auth: aBpk3v-iYsfdD8dGRVOmy2SH7vw Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: [FFmpeg-devel] [mov] Fix trampling of ctts during seeks when sidx support is enabled. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- libavformat/isom.h | 1 + libavformat/mov.c | 46 +++++++++++++++++++++++++++++++--------------- 2 files changed, 32 insertions(+), 15 deletions(-) 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); 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 =