From patchwork Tue Oct 25 15:38:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 1170 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.140.133 with SMTP id o127csp30833vsd; Tue, 25 Oct 2016 08:53:22 -0700 (PDT) X-Received: by 10.28.141.148 with SMTP id p142mr3708378wmd.107.1477410802692; Tue, 25 Oct 2016 08:53:22 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id x81si4380649wme.49.2016.10.25.08.53.16; Tue, 25 Oct 2016 08:53:22 -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=@gmail.com; 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 dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 11B60689D8C; Tue, 25 Oct 2016 18:53:11 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-yw0-f193.google.com (mail-yw0-f193.google.com [209.85.161.193]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8AF68689D59 for ; Tue, 25 Oct 2016 18:53:04 +0300 (EEST) Received: by mail-yw0-f193.google.com with SMTP id h14so636212ywa.2 for ; Tue, 25 Oct 2016 08:53:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:subject:date:message-id; bh=bSwX4wV5OLG9zczCYz+/kMjKbXkyvgh4D0CddF7u0sw=; b=Y4SQzuQO9tX1KZA8BuP5udpoYj8vnzy0BT4dt6Q9n9PciZzXOoPCwyhzdrb+21zkEB +TygEMPiqlouL2Tb98MNKzRkop5RFQFWIALWopWK/252UyVooQ1A4rLxb+Lt3VpoI77c iDFlWfwveGk/amMUvzwp/RfZM8ubQZJbo1h/QWmGu0PGLC3xPy+QAxj9rt8ydzQYLRlk KUqOg1SScrYJ5qDb1WbA+all9sUGwhzE8wDrS8dbXr9RNcHOu+58NnBBy7rQNVpbQsDB gSp1nj9JlvmSXTl8gT5iwnp/vgNqQXyzqgH4uwfOOZwUHvckork1O62PEQadRYknv0p7 SIXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=bSwX4wV5OLG9zczCYz+/kMjKbXkyvgh4D0CddF7u0sw=; b=UEqRB3T43349XYuudBSARv/pBVM7rEPB9FvPEAW+2JqqrQQruOCIvZ+jFCoPi+SCxz bknFoyeR/5U6AT+8U6xrE+gPhmlAYQGAdunQgUJEf0BMuWeExR26nEL+WK3yESgQil73 M+rLqAtyPB9w7h4KYp+iOlNk5+n/qJIpnYtEGbNZeJUpIVJG1PK5Ex0zmaxBaCoO7OVF GmxkC6MWSAl12aM5M0TcZfMt8XyH9Jqalo2Nu9Q1YnZ2r68NMzi4/3lMlbD3t2JXDCS0 +2b9I0Bk+qGV/OJvQjECumluTEQ7+uRZay9C3Ty4s57mxacsAN+n18TsoNfU1X8iGbaH GwqA== X-Gm-Message-State: ABUngvezVChZLaS8PeZoKVIUaMolgmEOcb8QPo1GvSB6kFhsu5W9GWHdAS3T+6PEq1qk/g== X-Received: by 10.13.203.197 with SMTP id n188mr24381448ywd.175.1477409914778; Tue, 25 Oct 2016 08:38:34 -0700 (PDT) Received: from localhost.localdomain ([181.22.25.77]) by smtp.gmail.com with ESMTPSA id d12sm7824901ywe.0.2016.10.25.08.38.32 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 25 Oct 2016 08:38:34 -0700 (PDT) From: James Almer To: ffmpeg-devel@ffmpeg.org Date: Tue, 25 Oct 2016 12:38:06 -0300 Message-Id: <20161025153806.6084-1-jamrial@gmail.com> X-Mailer: git-send-email 2.9.1 Subject: [FFmpeg-devel] [PATCH] avformat/matroskaenc: fix cue relative position values when CRC32 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 MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" The dynamic buffer does not contain the CRC32 element so calls to avio_tell() don't take it into account. This resulted in CueRelativePosition values being six bytes short. This is a regression since 6724525a1576ca334d2ffdc085620bb44aea7394 Instead of adding yet another custom check for CRC32 to fix a size or an offset, remove the existing ones and reserve the six bytes in the dynamic buffer. Signed-off-by: James Almer --- libavformat/matroskaenc.c | 39 +++++++++++++++++++++------------------ tests/ref/fate/rgb24-mkv | 2 +- tests/ref/lavf/mkv | 4 ++-- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 03d5326..d91055f 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -322,17 +322,19 @@ static void end_ebml_master(AVIOContext *pb, ebml_master master) avio_seek(pb, pos, SEEK_SET); } -static int start_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, ebml_master *master, - unsigned int elementid, uint64_t expectedsize) +static int start_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv, + ebml_master *master, unsigned int elementid, uint64_t expectedsize) { int ret; if ((ret = avio_open_dyn_buf(dyn_cp)) < 0) return ret; - if (pb->seekable) + if (pb->seekable) { *master = start_ebml_master(pb, elementid, expectedsize); - else + if (mkv->write_crc && mkv->mode != MODE_WEBM) + put_ebml_void(*dyn_cp, 6); /* Reserve space for CRC32 so position/size calculations using avio_tell() take it into account */ + } else *master = start_ebml_master(*dyn_cp, elementid, expectedsize); return 0; @@ -342,15 +344,16 @@ static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, Matrosk ebml_master master) { uint8_t *buf, crc[4]; - int size; + int size, skip = 0; if (pb->seekable) { size = avio_close_dyn_buf(*dyn_cp, &buf); if (mkv->write_crc && mkv->mode != MODE_WEBM) { - AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), UINT32_MAX, buf, size) ^ UINT32_MAX); + skip = 6; /* Skip reserved 6-byte long void element from the dynamic buffer. */ + AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), UINT32_MAX, buf + skip, size - skip) ^ UINT32_MAX); put_ebml_binary(pb, EBML_ID_CRC32, crc, sizeof(crc)); } - avio_write(pb, buf, size); + avio_write(pb, buf + skip, size - skip); end_ebml_master(pb, master); } else { end_ebml_master(*dyn_cp, master); @@ -465,7 +468,7 @@ static int64_t mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv) } } - if (start_ebml_master_crc32(pb, &dyn_cp, &metaseek, MATROSKA_ID_SEEKHEAD, + if (start_ebml_master_crc32(pb, &dyn_cp, mkv, &metaseek, MATROSKA_ID_SEEKHEAD, seekhead->reserved_size) < 0) { currentpos = -1; goto fail; @@ -541,7 +544,7 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra int i, j, ret; currentpos = avio_tell(pb); - ret = start_ebml_master_crc32(pb, &dyn_cp, &cues_element, MATROSKA_ID_CUES, 0); + ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, &cues_element, MATROSKA_ID_CUES, 0); if (ret < 0) return ret; @@ -1269,7 +1272,7 @@ static int mkv_write_tracks(AVFormatContext *s) if (ret < 0) return ret; - ret = start_ebml_master_crc32(pb, &dyn_cp, &tracks, MATROSKA_ID_TRACKS, 0); + ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, &tracks, MATROSKA_ID_TRACKS, 0); if (ret < 0) return ret; @@ -1300,7 +1303,7 @@ static int mkv_write_chapters(AVFormatContext *s) ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_CHAPTERS, avio_tell(pb)); if (ret < 0) return ret; - ret = start_ebml_master_crc32(pb, &dyn_cp, &chapters, MATROSKA_ID_CHAPTERS, 0); + ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, &chapters, MATROSKA_ID_CHAPTERS, 0); if (ret < 0) return ret; editionentry = start_ebml_master(dyn_cp, MATROSKA_ID_EDITIONENTRY, 0); @@ -1387,7 +1390,7 @@ static int mkv_write_tag_targets(AVFormatContext *s, ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_TAGS, avio_tell(s->pb)); if (ret < 0) return ret; - start_ebml_master_crc32(s->pb, &mkv->tags_bc, tags, MATROSKA_ID_TAGS, 0); + start_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, tags, MATROSKA_ID_TAGS, 0); } pb = mkv->tags_bc; @@ -1524,7 +1527,7 @@ static int mkv_write_tags(AVFormatContext *s) if (mkv->tags.pos) { if (s->pb->seekable && !mkv->is_live) - put_ebml_void(s->pb, avio_tell(mkv->tags_bc) + ((mkv->write_crc && mkv->mode != MODE_WEBM) ? 2 /* ebml id + data size */ + 4 /* CRC32 */ : 0)); + put_ebml_void(s->pb, avio_tell(mkv->tags_bc)); else end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, mkv->tags); } @@ -1551,7 +1554,7 @@ static int mkv_write_attachments(AVFormatContext *s) ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_ATTACHMENTS, avio_tell(pb)); if (ret < 0) return ret; - ret = start_ebml_master_crc32(pb, &dyn_cp, &attachments, MATROSKA_ID_ATTACHMENTS, 0); + ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, &attachments, MATROSKA_ID_ATTACHMENTS, 0); if (ret < 0) return ret; for (i = 0; i < s->nb_streams; i++) { @@ -1722,7 +1725,7 @@ static int mkv_write_header(AVFormatContext *s) ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_INFO, avio_tell(pb)); if (ret < 0) goto fail; - ret = start_ebml_master_crc32(pb, &mkv->info_bc, &mkv->info, MATROSKA_ID_INFO, 0); + ret = start_ebml_master_crc32(pb, &mkv->info_bc, mkv, &mkv->info, MATROSKA_ID_INFO, 0); if (ret < 0) return ret; pb = mkv->info_bc; @@ -1781,7 +1784,7 @@ static int mkv_write_header(AVFormatContext *s) } } if (s->pb->seekable) - put_ebml_void(s->pb, avio_tell(pb) + ((mkv->write_crc && mkv->mode != MODE_WEBM) ? 2 /* ebml id + data size */ + 4 /* CRC32 */ : 0)); + put_ebml_void(s->pb, avio_tell(pb)); else end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, mkv->info); pb = s->pb; @@ -2097,7 +2100,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_ if (mkv->cluster_pos == -1) { mkv->cluster_pos = avio_tell(s->pb); - ret = start_ebml_master_crc32(s->pb, &mkv->dyn_bc, &mkv->cluster, MATROSKA_ID_CLUSTER, 0); + ret = start_ebml_master_crc32(s->pb, &mkv->dyn_bc, mkv, &mkv->cluster, MATROSKA_ID_CLUSTER, 0); if (ret < 0) return ret; put_ebml_uint(mkv->dyn_bc, MATROSKA_ID_CLUSTERTIMECODE, FFMAX(0, ts)); @@ -2105,7 +2108,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_ } pb = mkv->dyn_bc; - relative_packet_pos = avio_tell(s->pb) - mkv->cluster.pos + avio_tell(pb); + relative_packet_pos = avio_tell(pb); if (par->codec_type != AVMEDIA_TYPE_SUBTITLE) { mkv_write_block(s, pb, MATROSKA_ID_SIMPLEBLOCK, pkt, keyframe); diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv index 19fd0e6..88d22c1 100644 --- a/tests/ref/fate/rgb24-mkv +++ b/tests/ref/fate/rgb24-mkv @@ -1,4 +1,4 @@ -7b8662e001bfb32a4bf709f4fe620138 *tests/data/fate/rgb24-mkv.matroska +94cce0d7d5b14b4c86e74a1ca454c5aa *tests/data/fate/rgb24-mkv.matroska 58361 tests/data/fate/rgb24-mkv.matroska #tb 0: 1/10 #media_type 0: video diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv index de161bf..0083033 100644 --- a/tests/ref/lavf/mkv +++ b/tests/ref/lavf/mkv @@ -1,6 +1,6 @@ -d23ff6ba071001256fa5073a0a528337 *./tests/data/lavf/lavf.mkv +7c8697c324e8ad79c5ea14364a6c39b8 *./tests/data/lavf/lavf.mkv 472759 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 -e33c89229c7d5014137c7217f4547467 *./tests/data/lavf/lavf.mkv +9767a3b526d7e56d7400164cb888990c *./tests/data/lavf/lavf.mkv 320603 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68