From patchwork Wed Apr 1 10:46:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18566 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 2A45E449F0D for ; Wed, 1 Apr 2020 13:46:53 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F24B468AFBC; Wed, 1 Apr 2020 13:46:52 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B29FD68AFA1 for ; Wed, 1 Apr 2020 13:46:46 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id u10so29960295wro.7 for ; Wed, 01 Apr 2020 03:46:46 -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:mime-version :content-transfer-encoding; bh=ws9Lc3e25lmWI5lPnI0NVM9WJ5/wRQDiD5k1r5yOdrM=; b=QV0FRkXu/6s2fKpOU+st1blSvGiTfs0igEKnMniQUk0Y5nSBuFBdm+7XpsYiXDkIqq HbS17BATMaA9DTWwYcjkECOhsk+T43FopNBAovSWlL63fU8IjKQBo6jfdjQUA/7Y4l9u /NiJqRexJI140weqCFF1HC1DNFs3P3AID4ojNRLA/InQ3Pf95N0mcEtfyGmUFv6Op8+R WUrpoX5DWmBaFM6jEwRyqpRqkArQdylxcQU5ttpNuTCoSKUUtsDwFUpuoT9MeTYsXHxY 4FvinG6HcE+2OcDmt0zFgwGacRv3mbDKKWcniGHUaxcUcmb1Wgq7GZ4aH844qWlsObZt HWgQ== 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=ws9Lc3e25lmWI5lPnI0NVM9WJ5/wRQDiD5k1r5yOdrM=; b=beT391F9GSRiKJLYwWldTwG6A2u3EwvleMJjNjSKlLKCIVhNTPqcwwSIBkkMPX9ww7 Q7b+k6QGkNk1WilIYoJZVvg0xTJ4TlLM3VZCgupo6IjUNtWlknBQSKxiAcRDQcVgD1fh 5KKgjhqECi6ugMtvCMjq1fvkZ8gq+OsAyNu14TcypANnN2QX3aWXlJXZS+NNbsst/HfF EKIpeab3DKEIceX5Z4nLjVe8BsUG/r1/Q4BtQwwme+amVPpW/JWy4euQQAsUs8xwpQsk Vxv9G1bye09TZlrmk+76uj4vvbSUCe7sPytRezqw4CrkDL3lcYj0pYN3japt0Gix5e7e LOfw== X-Gm-Message-State: ANhLgQ3P2tVN86fXMGw7aSagFP5EjE+ZumLZYgN8V4oVT+s/lCO8dCJR HRHMbpdyrf3cIy7w5Ytna9JDpOrL X-Google-Smtp-Source: ADFU+vsKrx7RQzip4hpHLdDV8884q6adyWbzk2xHyMPidIy9F6RRuLYFKkEoJ/mbJvDQoHRmCN9DMg== X-Received: by 2002:adf:8b5c:: with SMTP id v28mr23772764wra.98.1585738005748; Wed, 01 Apr 2020 03:46:45 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id w3sm2427800wrn.31.2020.04.01.03.46.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2020 03:46:44 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 1 Apr 2020 12:46:32 +0200 Message-Id: <20200401104634.25461-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 1/3] avformat/matroskaenc: Don't fail if reserved Cues space doesn't suffice 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 the user opted to write the Cues at the beginning, the Cues were simply written without checking in advance whether enough space has been reserved for them. If it wasn't enough, the data following the space reserved for the Cues was simply overwritten, corrupting the file. This commit changes this by checking whether enough space has been reserved for the Cues before outputting anything. If it isn't enough, no Cues will be output at all and the file will be finalized normally, yet writing the trailer will nevertheless return an error to notify the user that his wish of having Cues at the front of the file hasn't been fulfilled. This change opens new usecases for this option: It is now safe to use this option to e.g. record live streams or to use it when muxing the output of an expensive encoding, because when the reserved space turns out to be insufficient, one ends up with a file that just lacks Cues but is otherwise fine. Signed-off-by: Andreas Rheinhardt --- v2: Now returning an error if the space reserved for the Cues does not suffice. (The earlier version had the drawback that the return value did not inform them of failure to write the Cues at the front due to the insufficient space reserved.) Apart from that the file is finalized normally. I intend to apply this soon (tomorrow) if no one objects. doc/muxers.texi | 5 ++- libavformat/matroskaenc.c | 86 +++++++++++++++++++++------------------ 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/doc/muxers.texi b/doc/muxers.texi index d304181671..3be1c89416 100644 --- a/doc/muxers.texi +++ b/doc/muxers.texi @@ -1352,8 +1352,9 @@ index at the beginning of the file. If this option is set to a non-zero value, the muxer will reserve a given amount of space in the file header and then try to write the cues there when the muxing -finishes. If the available space does not suffice, muxing will fail. A safe size -for most use cases should be about 50kB per hour of video. +finishes. If the reserved space does not suffice, no Cues will be written, the +file will be finalized and writing the trailer will return an error. +A safe size for most use cases should be about 50kB per hour of video. Note that cues are only written if the output is seekable and this option will have no effect if it is not. diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 5dae53026d..22cc693720 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -500,23 +500,15 @@ static int mkv_add_cuepoint(MatroskaMuxContext *mkv, int stream, int tracknum, i return 0; } -static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tracks, int num_tracks) +static int mkv_assemble_cues(AVStream **streams, AVIOContext *dyn_cp, + mkv_cues *cues, mkv_track *tracks, int num_tracks) { - MatroskaMuxContext *mkv = s->priv_data; - AVIOContext *dyn_cp, *pb = s->pb, *cuepoint; - int64_t currentpos; + AVIOContext *cuepoint; int ret; - currentpos = avio_tell(pb); - ret = start_ebml_master_crc32(&dyn_cp, mkv); - if (ret < 0) - return ret; - ret = avio_open_dyn_buf(&cuepoint); - if (ret < 0) { - ffio_free_dyn_buf(&dyn_cp); + if (ret < 0) return ret; - } for (mkv_cuepoint *entry = cues->entries, *end = entry + cues->num_entries; entry < end;) { @@ -535,7 +527,7 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra int idx = entry->stream_idx; av_assert0(idx >= 0 && idx < num_tracks); - if (tracks[idx].has_cue && s->streams[idx]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE) + if (tracks[idx].has_cue && streams[idx]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE) continue; tracks[idx].has_cue = 1; track_positions = start_ebml_master(cuepoint, MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE); @@ -551,9 +543,8 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra ffio_reset_dyn_buf(cuepoint); } ffio_free_dyn_buf(&cuepoint); - end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CUES); - return currentpos; + return 0; } static int put_xiph_codecpriv(AVFormatContext *s, AVIOContext *pb, AVCodecParameters *par) @@ -2464,7 +2455,7 @@ static int mkv_write_trailer(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; AVIOContext *pb = s->pb; - int64_t currentpos, cuespos; + int64_t currentpos; int ret; // check if we have an audio packet cached @@ -2487,35 +2478,52 @@ static int mkv_write_trailer(AVFormatContext *s) if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) { + int64_t ret64; + if (mkv->cues.num_entries) { - if (mkv->reserve_cues_space) { - int64_t cues_end; - - currentpos = avio_tell(pb); - avio_seek(pb, mkv->cues_pos, SEEK_SET); - - cuespos = mkv_write_cues(s, &mkv->cues, mkv->tracks, s->nb_streams); - cues_end = avio_tell(pb); - if (cues_end > cuespos + mkv->reserve_cues_space) { - av_log(s, AV_LOG_ERROR, - "Insufficient space reserved for cues: %d " - "(needed: %" PRId64 ").\n", - mkv->reserve_cues_space, cues_end - cuespos); - return AVERROR(EINVAL); - } + AVIOContext *cues; + uint64_t size; - if (cues_end < cuespos + mkv->reserve_cues_space) - put_ebml_void(pb, mkv->reserve_cues_space - - (cues_end - cuespos)); + ret = start_ebml_master_crc32(&cues, mkv); + if (ret < 0) + return ret; - avio_seek(pb, currentpos, SEEK_SET); - } else { - cuespos = mkv_write_cues(s, &mkv->cues, mkv->tracks, s->nb_streams); + ret = mkv_assemble_cues(s->streams, cues, &mkv->cues, + mkv->tracks, s->nb_streams); + if (ret < 0) { + ffio_free_dyn_buf(&cues); + return ret; } - mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, cuespos); + if (mkv->reserve_cues_space) { + size = avio_tell(cues); + size += 4 + ebml_num_size(size); + if (mkv->reserve_cues_space < size) { + av_log(s, AV_LOG_WARNING, + "Insufficient space reserved for Cues: " + "%d < %"PRIu64". No Cues will be output.\n", + mkv->reserve_cues_space, size); + mkv->reserve_cues_space = -1; + ffio_free_dyn_buf(&cues); + goto after_cues; + } else { + currentpos = avio_tell(pb); + if ((ret64 = avio_seek(pb, mkv->cues_pos, SEEK_SET)) < 0) { + ffio_free_dyn_buf(&cues); + return ret64; + } + } + } + mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, avio_tell(pb)); + end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES); + if (mkv->reserve_cues_space) { + if (size < mkv->reserve_cues_space) + put_ebml_void(pb, mkv->reserve_cues_space - size); + avio_seek(pb, currentpos, SEEK_SET); + } } + after_cues: currentpos = avio_tell(pb); ret = mkv_write_seekhead(pb, mkv, 1, mkv->info_pos); @@ -2570,7 +2578,7 @@ static int mkv_write_trailer(AVFormatContext *s) end_ebml_master(pb, mkv->segment); } - return 0; + return mkv->reserve_cues_space < 0 ? AVERROR(EINVAL) : 0; } static int mkv_query_codec(enum AVCodecID codec_id, int std_compliance)