From patchwork Mon Mar 8 15:55:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derek Buitenhuis X-Patchwork-Id: 26274 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 5613B44A439 for ; Mon, 8 Mar 2021 18:03:57 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 24B4868059A; Mon, 8 Mar 2021 18:03:57 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E431668059A for ; Mon, 8 Mar 2021 18:03:50 +0200 (EET) Received: by mail-wm1-f42.google.com with SMTP id r10-20020a05600c35cab029010c946c95easo4126999wmq.4 for ; Mon, 08 Mar 2021 08:03:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=QM4l0KjQpmwnb8HJdVyNwzkvzdadq8GrIEuUscL/MLo=; b=IsStEHwWulaUbVCTlPshgVtk/uAIGLf/iewJF51baOShFD9PpaBfbeXEB+9Cb5e6C0 ddXYuDuRF4N8at4jpSSUz+lPJijzpoOzEMk7bXrzj+msDO9qByOSJu9kE892OuHJ3oCI JhiF0XAQRGKYteljGfNLreK3fMQfUUH45GgYH9ltl4rOwDVLYoIqJYOa+SKjY7OivF/R ayDBz67h58gjTo+d3Pp+vPV2kR+h4CGvrlu0L+0MtZSqJKwqr3pPwMnqwzSAhDZooRnn zArs6gRZGsEvEDgcRXCGUh2V+jYc12fZiVU1H1SFr1y4k0y26mBhtQdYwomOMlo1X5kN 8V4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=QM4l0KjQpmwnb8HJdVyNwzkvzdadq8GrIEuUscL/MLo=; b=PLinJG/SGevATIW5XaRizkf3+j8H6GokIVJYoQcVaes791I7ZTM/tROAvZB3bhO4ZU 4RAxWcUKVhiQdHvoadBsU/NKZ9LEmhjI0Gwp/IiRaO1s+y3r8nm+af9tvFaHkhwmX5Am 3yKV1kSTkvyXUrKUHL/JIIPJU6SsSiBnEKNmaMJ9rwqqzsThzdHkb7z8+Yzg69gyjQQX gvWPiu/ZEv0Sluk5vGYkz35vv6SzTjueeGWS5qCUaGD9dtasc4sJXT/KTlivede11P8J BrAYy+8SM324FTeqo1CreUEooNAqXLCmghYk+o0RjTpxJ8EoqGYVmqgTeS2qLHbWzT2k Al4w== X-Gm-Message-State: AOAM5300E6Y/hhdGRRqhDqqv/iVuFLBAITCmRD8YhAJfK0kmqWHF7C31 CHUpnTOBc8KQupieeUuY2bX6N7NWYlY= X-Google-Smtp-Source: ABdhPJynGR8IA9D139+HHTebCwqK2RJdPSrdfKmjiJfAC6wz/qW0NMw3YtZtTF4qMRMxhoPAQz00dA== X-Received: by 2002:a1c:4802:: with SMTP id v2mr23285901wma.139.1615218929820; Mon, 08 Mar 2021 07:55:29 -0800 (PST) Received: from localhost.localdomain ([82.129.110.36]) by smtp.gmail.com with ESMTPSA id p16sm22456789wrt.54.2021.03.08.07.55.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Mar 2021 07:55:29 -0800 (PST) From: Derek Buitenhuis To: ffmpeg-devel@ffmpeg.org Date: Mon, 8 Mar 2021 15:55:19 +0000 Message-Id: <20210308155519.807360-1-derek.buitenhuis@gmail.com> X-Mailer: git-send-email 2.30.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH][RFC - DO NOT MERGE] Revert "mov: Discard invalid CTTS." 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 Cc: michael@niedermayer.cc Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" This reverts commit 4093220029a4d77f272c491e9299680480a08c00. Signed-off-by: Derek Buitenhuis --- I have CC'd Michael here, as he is the original author here and the "soltuion" is nor clear. To explain this RFC: We (Vimeo) have started seeing an uptick in broken MP4 files (what creates them is under investigation; we have reached out to some users in hopes that we can identify the broken muxer and contact its authors). They are broken in a very specific way: The CTTS box starts off normal, but then at some point, the duration field of the CTTS entries starts to increment on each entry, and this continues until the end of the file, resulting in incorrect and insane PTS/DTS differences (like PTS/DTS differing by 15 minutes / 10,000 frames). I have linked both a text dump of the boxes and a trimmed moov box as an example in [1] and [2]. I can only provide a full file (~12 GiB) privately. Thes files *happen* to decode linearly just fine, of course, because FFmpeg doesn't care about the CTTS info in that case, but if you seek, everything will explode, as you would expect. So, anyway, in the meanwhile, I was writing a simple detection filter for these sorts of files, and I noticed it only detected some of them, and this lead me to 1fb9efbda0149c3491529ea5fa92cfdcb8cebfaa. What happens is that if the file is long enough, the incrementing CTTS entry durations eventually trigger this, as far as I can tell, totally arbitrary check for 1<<28, and that results in the entire CTTS being thrown away, and libavformat setting PTS==DTS for all packets, which both makes detection impossible, and breaks seeking in a completely different way. So the options are: * Revert 1fb9efbda0149c3491529ea5fa92cfdcb8cebfaa and detect them as I was already. * Change the check to something else or some other value, which would still be totally arbitrary - but maybe something like >16 frames. This would still leave seeking totally broken, of course, since it will still set PTS==DTS for all packets. This is still maybe detectable for API users by checking b_frames and has_b_frames of video_delay, but PTS==DTS? Seems kind gross, but... eh. * (Possibly in addition to other changes) Make the check an error, or make seeking return an error when this sort of file is detected. * Something I haven't listed here. So, basically, all options suck, and I thought some people may have opinions on the least bad option, here - specifically Michael, as the author of the original patch. People: Please comment if you feel strongly about one solution or another. [1] http://chromashift.org/ctts_sadness/broken_cut.mp4.xz [2] http://chromashift.org/ctts_sadness/broken.boxes.txt.xz --- libavformat/mov.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 1c07cff6b5..256e7d376f 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3079,13 +3079,6 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) av_log(c->fc, AV_LOG_TRACE, "count=%d, duration=%d\n", count, duration); - if (FFNABS(duration) < -(1<<28) && i+2fc, AV_LOG_WARNING, "CTTS invalid\n"); - av_freep(&sc->ctts_data); - sc->ctts_count = 0; - return 0; - } - if (i+2fc); }