From patchwork Thu May 16 22:30:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 13157 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 8A96C44964F for ; Fri, 17 May 2019 01:43:18 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 71469689BF3; Fri, 17 May 2019 01:43:18 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 99AE5689C3E for ; Fri, 17 May 2019 01:43:16 +0300 (EEST) Received: by mail-wm1-f66.google.com with SMTP id 15so910872wmg.5 for ; Thu, 16 May 2019 15:43:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=bHbn1YljZiaPtNgl0b/7B+hlYt6lBHbM+cnsIiqQ4U4=; b=A3qXKWPZLNRlewMA2hfymrrQLY8Ai2i284WyRcJGaxgFVzEvUUKeYYjd9pqBU/j8/e g3f4yQ6i4syLcA4K0sZxS5xVMTEgkQyVS82whhRhgK0gztf0jHyHTzrDWQdUjewa7akv t9DDTOe+rH8Mnj9zCB/OnFnFsD7lDGfPeSu7oxs9TsDCFIvtwXJR6z5D+x+8ObTb7lKR RYVZ9wc+QkUlSFu2nvDQQm1N4w24/+zdqOzzBtlNXHJcO+IOXOMMP1cVm21XqswJbj0/ YFZx/2LBPovIEXdm/zW8LpAGhDESV+zPTjrFCqKpBtfLBxLTnxwjKZIj+OeLqw7byg5+ w6hQ== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=bHbn1YljZiaPtNgl0b/7B+hlYt6lBHbM+cnsIiqQ4U4=; b=tBmK6uAWQzUgdr9Cekrpfi6ETtadxddoo6BejT/1kGF6iG71WIi+dnGXnSd2n3pnpO OX+wMCkQ0zW0buB8NgjGpAx0mDXmlF4KlFCfD5z3E8LqHf1hHaQRb7uQTuON1CKhkGzV D+1QUcDyLrVuMo6TBLSkA9eaUzng6Et1DYLr4F/n9yFCxLM6vke6vkuxEwlwE2kMJYyC 0mK5b0Y0GA5KPbo8b3gZT7wiC/aL8DFnwaJCeDtrLmgRL0puK60RmauCBboQ6z4JCNvu LN3F9nELeLexJwebasGLdxoKSoK8/lRxco2LpfTkVgk8LLGHwYE9QF9/QpdhLaYiXZOQ MGuw== X-Gm-Message-State: APjAAAVDKPoRwZPcoQCnVFxkkkQvcHSCJPop7nSoU0X8kdDHAHHjbQtN xd2w5Rf5wtbnPRpovoqEEhJfSmIM X-Google-Smtp-Source: APXvYqy5c9J4JspM9KUNPZowhlCb/kJTRU+yct4llE+9rX4UurLwsromZEl1RIDr7LeIviODQtkfdg== X-Received: by 2002:a1c:5f02:: with SMTP id t2mr75813wmb.19.1558046595900; Thu, 16 May 2019 15:43:15 -0700 (PDT) Received: from localhost.localdomain (ipbcc18715.dynamic.kabel-deutschland.de. [188.193.135.21]) by smtp.gmail.com with ESMTPSA id i185sm11168725wmg.32.2019.05.16.15.43.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 May 2019 15:43:15 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 17 May 2019 00:30:01 +0200 Message-Id: <20190516223018.30827-18-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190516223018.30827-1-andreas.rheinhardt@gmail.com> References: <20190516223018.30827-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 17/37] avformat/matroskadec: Don't abort resyncing upon seek failure 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: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" When an error happens, the Matroska demuxer tries to resync to level 1 elements from an earlier position onwards. If the seek to said earlier position fails, the demuxer currently treats this as an unrecoverable error. And that behaviour is suboptimal as said failure is nothing unrecoverable or unexpected (when the input isn't seekable). It is preferable to simply resync from the earliest position available (i.e. the start of the AVIOContext's buffer) onwards if the seek failed. Here are some scenarios that might be treated as unrecoverable errors by the current code if the input isn't seekable. They all have in common that the current position is so far away from the desired position that the seek can't be fulfilled from the AVIOContext's buffer: 1. Blocks (both SimpleBlocks as well as a Block in a BlockGroup) for which reading them as binary EBML elements succeeds, but whose parsing triggers an error (e.g. an invalid TrackNumber). In this case the earlier position from which resyncing begins is at the start of the block (or even earlier). 2. BlockGroups, whose parsing fails in one of the latter elements. Just as in 1., the start of the BlockGroup (the target of the seek) might be so far away from the current position that it is no longer in the buffer. 3. At the beginning of parsing a cluster, the cluster is parsed until a SimpleBlock or a BlockGroup is encountered. So if the input is damaged between the beginning of the cluster and the first occurrence of a SimpleBlock/BlockGroup and if said damage makes the demuxer read/skip so much data that the beginning of the cluster is no longer in the buffer, demuxing will currently fail completely. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 93bf72661c..b14f168c2c 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -742,15 +742,18 @@ static int matroska_read_close(AVFormatContext *s); static int matroska_resync(MatroskaDemuxContext *matroska, int64_t last_pos) { AVIOContext *pb = matroska->ctx->pb; - int64_t ret; uint32_t id; matroska->current_id = 0; matroska->num_levels = 0; - /* seek to next position to resync from */ - if ((ret = avio_seek(pb, last_pos + 1, SEEK_SET)) < 0) { - matroska->done = 1; - return ret; + /* Try to seek to the last position to resync from. If this doesn't work, + * we resync from the earliest position available: The start of the buffer. */ + if (last_pos < avio_tell(pb) && avio_seek(pb, last_pos + 1, SEEK_SET) < 0) { + av_log(matroska->ctx, AV_LOG_WARNING, + "Seek to desired resync point failed. Seeking to " + "earliest point available instead.\n"); + avio_seek(pb, FFMAX(avio_tell(pb) + (pb->buffer - pb->buf_ptr), + last_pos + 1), SEEK_SET); } id = avio_rb32(pb); @@ -768,7 +771,7 @@ static int matroska_resync(MatroskaDemuxContext *matroska, int64_t last_pos) } matroska->done = 1; - return AVERROR_EOF; + return pb->error ? pb->error : AVERROR_EOF; } /*