From patchwork Wed Apr 29 22:21:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19383 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 02A5944B57C for ; Thu, 30 Apr 2020 01:22:13 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D57C768C1E3; Thu, 30 Apr 2020 01:22:12 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 69D4868C099 for ; Thu, 30 Apr 2020 01:22:06 +0300 (EEST) Received: by mail-wm1-f67.google.com with SMTP id x25so3790965wmc.0 for ; Wed, 29 Apr 2020 15:22:06 -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=clMGnfpnRuvDrIhFOXO8HXqGPGt2ZIDAkONCpoJj9JM=; b=WOThSfOuAWdWptLHB7My6ZN4YPFj2BEeE7MfL62k/ENlObja6FJ/qKn+6KQcX9bR9l ssqlUVpUlIoKlDa7lA9MYsL5e5N3APQPYFwiDyrOdasZRODA7iIYx+04lHRvOGk03PnY bpA9Os3uVxyYW2Y/AEooe9U55NVwbp4s40KSQm37vHKgotc5/TYFY+O644Lvpf443X0I okDkZ0beBZr+eO2f4pDnhAASWqZcIyOS7MNz6XxxQY7Y3FId2zi3EreDwW+OYb+0sitj sLZfYaGjkqKjXT9zTICMA1DGNHJsVXCTdLxgMcdXcgKVBmm5L492h2zTVtSO/HdwvgyT 2m0A== 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=clMGnfpnRuvDrIhFOXO8HXqGPGt2ZIDAkONCpoJj9JM=; b=riyj9bmeaNQ/PlpewmPnDuIsjHFKDAKy9n/M1Jdf5qXMY5E0viGU3dTAfFu8MexL1z gIw0XMdpAp0G4RaRyB8cXtqyrKKGVKBbiK6Bk9xi9yDVUTMdycaaQA245FgI13PF8ede ypAJhR3vxU434lYWZAQw/0qKSc2vPZHmHW9jBNgDfT+CYbrrIECKdL5JBWIkYeOr6RhA HhmTv0vObajPImmiFciSmDVQgXBAq8OVWES/3pnvhLue4UugCg2WUYIzOdtQv8Ikzfrb mECX7a/QiOwlUgqdIsOMVwQ08q5bmnsxzRlJnpcR6+C1nicdsZpakM3TBBvFZY3N9cBT aaCg== X-Gm-Message-State: AGi0PuZ4OJ2viPRyK2R/EJx1VZfEXCmzz5KBquiJYWdId4PvFAu+hrKT O4mCfOaqJCVh1ur3cZR95k7FvPh8 X-Google-Smtp-Source: APiQypJzadrBKAdXUQJdCBbBwnOqFRF67DWrbGJO157cxoBHnAghZlarzSP+5lzVWGcyMBgl5NFZuw== X-Received: by 2002:a1c:7f91:: with SMTP id a139mr42634wmd.164.1588198925397; Wed, 29 Apr 2020 15:22:05 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id x6sm1071555wrg.58.2020.04.29.15.22.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Apr 2020 15:22:04 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Thu, 30 Apr 2020 00:21:51 +0200 Message-Id: <20200429222156.29129-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/6] avformat/matroskaenc: Move adding SeekEntry into end_ebml_master_crc32() 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" Up until now, SeekEntries were already added before start_ebml_master_crc32() was even called and before we were actually sure that we really write the element the SeekHead references; after all, we might also error out later; and given that the allocations implicit in dynamic buffers should be checked, end_ebml_master_crc32() will eventually have to return errors itself, so that it is the right place to add SeekHead entries. The earlier behaviour is of course a remnant of the time in which start_ebml_master_crc32() really did output something, so that the position before start_ebml_master_crc32() needed to be recorded. Erroring out later is also not as dangerous as it seems because in this case no SeekHead will be written (if it happened when writing the header, the whole muxing process would abort; if it happened when writing the trailer (when writing chapters not available initially), writing the trailer would be aborted and no SeekHead containing the bogus chapter entry would be written). This commit does not change the way the SeekEntries are added for those elements that are output preliminarily; this is so because the SeekHead is written before those elements are finally output and doing it otherwise would increase the amount of seeks. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskaenc.c | 64 ++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index a1b613290c..b50fd8dd9b 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -349,6 +349,17 @@ static void end_ebml_master(AVIOContext *pb, ebml_master master) avio_seek(pb, pos, SEEK_SET); } +static void mkv_add_seekhead_entry(MatroskaMuxContext *mkv, uint32_t elementid, + uint64_t filepos) +{ + mkv_seekhead *seekhead = &mkv->seekhead; + + av_assert1(seekhead->num_entries < MAX_SEEKHEAD_ENTRIES); + + seekhead->entries[seekhead->num_entries].elementid = elementid; + seekhead->entries[seekhead->num_entries++].segmentpos = filepos - mkv->segment_offset; +} + static int start_ebml_master_crc32(AVIOContext **dyn_cp, MatroskaMuxContext *mkv) { int ret; @@ -364,11 +375,15 @@ static int start_ebml_master_crc32(AVIOContext **dyn_cp, MatroskaMuxContext *mkv static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv, uint32_t id, - int length_size, int keep_buffer) + int length_size, int keep_buffer, + int add_seekentry) { uint8_t *buf, crc[4]; int size, skip = 0; + if (add_seekentry) + mkv_add_seekhead_entry(mkv, id, avio_tell(pb)); + put_ebml_id(pb, id); size = avio_get_dyn_buf(*dyn_cp, &buf); put_ebml_length(pb, size, length_size); @@ -441,17 +456,6 @@ static void mkv_start_seekhead(MatroskaMuxContext *mkv, AVIOContext *pb) put_ebml_void(pb, mkv->seekhead.reserved_size); } -static void mkv_add_seekhead_entry(MatroskaMuxContext *mkv, uint32_t elementid, - uint64_t filepos) -{ - mkv_seekhead *seekhead = &mkv->seekhead; - - av_assert1(seekhead->num_entries < MAX_SEEKHEAD_ENTRIES); - - seekhead->entries[seekhead->num_entries].elementid = elementid; - seekhead->entries[seekhead->num_entries++].segmentpos = filepos - mkv->segment_offset; -} - /** * Write the SeekHead to the file at the location reserved for it * and seek to destpos afterwards. When error_on_seek_failure @@ -489,7 +493,7 @@ static int mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv, put_ebml_uint(dyn_cp, MATROSKA_ID_SEEKPOSITION, entry->segmentpos); end_ebml_master(dyn_cp, seekentry); } - end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD, 0, 0); + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD, 0, 0, 0); remaining = seekhead->filepos + seekhead->reserved_size - avio_tell(pb); put_ebml_void(pb, remaining); @@ -1421,7 +1425,8 @@ static int mkv_write_tracks(AVFormatContext *s) end_ebml_master_crc32_preliminary(pb, mkv->tracks_bc, MATROSKA_ID_TRACKS, &mkv->tracks_pos); else - end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS, 0, 0); + end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, + MATROSKA_ID_TRACKS, 0, 0, 0); return 0; } @@ -1443,8 +1448,6 @@ static int mkv_write_chapters(AVFormatContext *s) break; } - mkv_add_seekhead_entry(mkv, MATROSKA_ID_CHAPTERS, avio_tell(pb)); - ret = start_ebml_master_crc32(&dyn_cp, mkv); if (ret < 0) return ret; @@ -1481,7 +1484,7 @@ static int mkv_write_chapters(AVFormatContext *s) end_ebml_master(dyn_cp, chapteratom); } end_ebml_master(dyn_cp, editionentry); - end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS, 0, 0); + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS, 0, 0, 1); mkv->wrote_chapters = 1; return 0; @@ -1682,7 +1685,8 @@ static int mkv_write_tags(AVFormatContext *s) end_ebml_master_crc32_preliminary(s->pb, mkv->tags_bc, MATROSKA_ID_TAGS, &mkv->tags_pos); else - end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS, 0, 0); + end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, + MATROSKA_ID_TAGS, 0, 0, 0); } return 0; } @@ -1713,8 +1717,6 @@ static int mkv_write_attachments(AVFormatContext *s) if (!mkv->nb_attachments) return 0; - mkv_add_seekhead_entry(mkv, MATROSKA_ID_ATTACHMENTS, avio_tell(pb)); - ret = start_ebml_master_crc32(&dyn_cp, mkv); if (ret < 0) return ret; @@ -1746,7 +1748,7 @@ static int mkv_write_attachments(AVFormatContext *s) put_ebml_uid(dyn_cp, MATROSKA_ID_FILEUID, track->uid); end_ebml_master(dyn_cp, attached_file); } - end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS, 0, 0); + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS, 0, 0, 1); return 0; } @@ -1868,7 +1870,8 @@ static int mkv_write_header(AVFormatContext *s) end_ebml_master_crc32_preliminary(s->pb, mkv->info_bc, MATROSKA_ID_INFO, &mkv->info_pos); else - end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO, 0, 0); + end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, + MATROSKA_ID_INFO, 0, 0, 0); pb = s->pb; ret = mkv_write_tracks(s); @@ -2153,7 +2156,8 @@ static void mkv_end_cluster(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; - end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER, 0, 1); + end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, + MATROSKA_ID_CLUSTER, 0, 1, 0); if (!mkv->have_video) { for (unsigned i = 0; i < s->nb_streams; i++) mkv->tracks[i].has_cue = 0; @@ -2447,7 +2451,7 @@ static int mkv_write_trailer(AVFormatContext *s) if (mkv->cluster_bc) { end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv, - MATROSKA_ID_CLUSTER, 0, 0); + MATROSKA_ID_CLUSTER, 0, 0, 0); } ret = mkv_write_chapters(s); @@ -2463,7 +2467,6 @@ static int mkv_write_trailer(AVFormatContext *s) if (mkv->cues.num_entries) { AVIOContext *cues = NULL; uint64_t size; - int64_t cuespos = endpos; int length_size = 0; ret = start_ebml_master_crc32(&cues, mkv); @@ -2490,7 +2493,6 @@ static int mkv_write_trailer(AVFormatContext *s) ffio_free_dyn_buf(&cues); goto after_cues; } else { - cuespos = mkv->cues_pos; if ((ret64 = avio_seek(pb, mkv->cues_pos, SEEK_SET)) < 0) { ffio_free_dyn_buf(&cues); return ret64; @@ -2506,9 +2508,8 @@ static int mkv_write_trailer(AVFormatContext *s) } } } - mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, cuespos); end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES, - length_size, 0); + length_size, 0, 1); if (mkv->reserve_cues_space) { if (size < mkv->reserve_cues_space) put_ebml_void(pb, mkv->reserve_cues_space - size); @@ -2525,13 +2526,13 @@ static int mkv_write_trailer(AVFormatContext *s) av_log(s, AV_LOG_DEBUG, "end duration = %" PRIu64 "\n", mkv->duration); avio_seek(mkv->info_bc, mkv->duration_offset, SEEK_SET); put_ebml_float(mkv->info_bc, MATROSKA_ID_DURATION, mkv->duration); - end_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO, 0, 0); + end_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO, 0, 0, 0); if (mkv->tracks_bc) { // write Tracks master avio_seek(pb, mkv->tracks_pos, SEEK_SET); end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, - MATROSKA_ID_TRACKS, 0, 0); + MATROSKA_ID_TRACKS, 0, 0, 0); } // update stream durations @@ -2559,7 +2560,8 @@ static int mkv_write_trailer(AVFormatContext *s) } avio_seek(pb, mkv->tags_pos, SEEK_SET); - end_ebml_master_crc32(pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS, 0, 0); + end_ebml_master_crc32(pb, &mkv->tags_bc, mkv, + MATROSKA_ID_TAGS, 0, 0, 0); } avio_seek(pb, endpos, SEEK_SET);