From patchwork Tue Jan 3 23:49:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Soft Works X-Patchwork-Id: 2038 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp4590064vsb; Tue, 3 Jan 2017 15:49:22 -0800 (PST) X-Received: by 10.28.15.5 with SMTP id 5mr59600024wmp.141.1483487362215; Tue, 03 Jan 2017 15:49:22 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id q14si75366500wmd.13.2017.01.03.15.49.21; Tue, 03 Jan 2017 15:49:22 -0800 (PST) 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=@hotmail.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=hotmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 444CD689E72; Wed, 4 Jan 2017 01:49:14 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from COL004-OMC3S11.hotmail.com (col004-omc3s11.hotmail.com [65.55.34.149]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E1183689E4A for ; Wed, 4 Jan 2017 01:49:06 +0200 (EET) Received: from NAM02-BL2-obe.outbound.protection.outlook.com ([65.55.34.137]) by COL004-OMC3S11.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.23008); Tue, 3 Jan 2017 15:49:09 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=PV5iKByFAH/FyEptclrLODK5zM5eG7wZyoNlQFOFNu8=; b=fmEScLVUK9fP3xp/UGuO9Cnjmu/LlUm+fvyy+31NZfECYs9fg7DlQZvT4OWFUJ0ypRjBOOgAhSIIXnJZUQ98hJ3WSO7Zvw3SuVDjho4i9Sr7vKNtjTRtdG3j+fmPTfhaOVYNNmSMhjdPNNip+gkQIxOAZcYvDCE1IUAp0XLRT76SHzilVWJAH2MOLAm0/WNO21w4IyCmWONfcpyEcwsKZ7Tlj0GdH9HWAs38MPypwnwJaoJeA5VfrTtv8Zuu/XrPI0t5tuILUhR9Y7AxFreiA1woTZ/S94ESMTgguclO2kn+qwfUo7PKof5zyL5eGUg6kqmA/dRm0qtmT5izXxEMpA== Received: from CY1NAM02FT015.eop-nam02.prod.protection.outlook.com (10.152.74.52) by CY1NAM02HT054.eop-nam02.prod.protection.outlook.com (10.152.74.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.803.8; Tue, 3 Jan 2017 23:49:07 +0000 Received: from MWHPR13MB1678.namprd13.prod.outlook.com (10.152.74.51) by CY1NAM02FT015.mail.protection.outlook.com (10.152.75.146) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.803.8 via Frontend Transport; Tue, 3 Jan 2017 23:49:08 +0000 Received: from MWHPR13MB1678.namprd13.prod.outlook.com ([10.171.144.16]) by MWHPR13MB1678.namprd13.prod.outlook.com ([10.171.144.16]) with mapi id 15.01.0829.003; Tue, 3 Jan 2017 23:49:08 +0000 From: Soft Works To: "ffmpeg-devel@ffmpeg.org" Thread-Topic: [PATCH] avformat/matroskaenc: Regression fix for invalid MKV headers Thread-Index: AQHSZhv0Z13NEadwxUaLCJ6sf0vGPw== Date: Tue, 3 Jan 2017 23:49:07 +0000 Message-ID: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: authentication-results: ffmpeg.org; dkim=none (message not signed) header.d=none; ffmpeg.org; dmarc=none action=none header.from=hotmail.com; x-incomingtopheadermarker: OriginalChecksum:6428623628A677565DE2D952D78FA66246F6DCEA4796631C81EF820E1070B2BC; UpperCasedChecksum:5D2FE2AC06105717EC1A0860BB9E02206ACF28C9D50A44BC5540A15C69073C17; SizeAsReceived:7296; Count:37 x-ms-exchange-messagesentrepresentingtype: 1 x-tmn: [jE4v01+ayeVbd2FP1ZNhkZVDzTjayRKt] x-incomingheadercount: 37 x-eopattributedmessage: 0 x-microsoft-exchange-diagnostics: 1; CY1NAM02HT054; 7:GgxWqH+8dCuXINNXWzl8hyw37amqdJWi6myRbFJlPY+LTKHgMEMJUXXw/fCqrZKVecF0pzXZR6mutvfUsQ/HaBkdITeEKF32yczmik7zXEKBAsgJDItiUcUG6sx1j18Btp8idfUHTMkE/0UoxsZJ8wtAti6OsAxe9wbCeSrOIaDSQVve4pt8HpZOGxTyYPAib8paZe7u2CeqvHPiUckTCOOmj+pgzjwhFuVwVsuuzDbFzCe7V0ziP1UY6pkkD6fD89BLYNvqFX7FLW+Ye37xid1+cC9sUg9aK/S/q60ZWTDnrcOqVdWl2smrejzgOyLhfsvLz8uQB4CA+eELEsucZl3LannVN0m/iF/r2yTUalQKfJd17u4rOq/6g36t1A9a8w1jDhFiKC+BxxIotvaNQ5JLzB/38CMP3lEo4NxTLOwDhxjDsBWD7TNW7+M+NF4s0TR4dP6mTXwvBCQyzBozOQ== x-forefront-antispam-report: EFV:NLI; SFV:NSPM; SFS:(10019020)(98900003); DIR:OUT; SFP:1102; SCL:1; SRVR:CY1NAM02HT054; H:MWHPR13MB1678.namprd13.prod.outlook.com; FPR:; SPF:None; LANG:en; x-ms-office365-filtering-correlation-id: 4aac65c2-0030-4eb9-2fe8-08d434331bd0 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(1601124038)(1603103113)(1603101340)(1601125047); SRVR:CY1NAM02HT054; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(444111334)(444112120)(432015012)(102415395)(82015046); SRVR:CY1NAM02HT054; BCL:0; PCL:0; RULEID:; SRVR:CY1NAM02HT054; x-forefront-prvs: 01762B0D64 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: hotmail.com X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jan 2017 23:49:07.8926 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1NAM02HT054 X-OriginalArrivalTime: 03 Jan 2017 23:49:09.0696 (UTC) FILETIME=[FA517C00:01D2661B] Subject: [FFmpeg-devel] [PATCH] avformat/matroskaenc: Regression fix for invalid MKV headers 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" The following three commits created a regression by writing initially invalid mkv headers: 650e17d88b63b5aca6e0a43483e89e64b0f7d2dd avformat/matroskaenc: write a CRC32 element on Tags 3bcadf822711720ff0f8d14db71ae47cdf97e652 avformat/matroskaenc: write a CRC32 element on Info ee888cfbe777cd2916a3548c750e433ab8f8e6a5 avformat/matroskaenc: postpone writing the Tracks master Symptoms: - You can no longer playback a file that is still processed by ffmpeg, e.g. VLC fails playback - You can no longer stream a file to a client while if is still being processed - Various diagnosing tools show header errors or incomplete headers (e.g. ffprobe, mediainfo, mkvalidator) Note: The symptoms do not apply to completed files or ffmpeg runs that were interrupted with 'q' Cause: The mentioned commits made changes in a way that some header elements are only partially written in mkv_write_header, leaving the header in an invalid state. Only in mkv_write_trailer, these elements are finished correctly, but that does only occur at the end of the process. Regression: Before these commits were applied, mkv headers have always been valid, even before completion of ffmpeg. This has worked reliably over many versions of ffmpeg, to it was an obvious regression. Bugtracker: This issue has been recorded as #5977 which is resolved by this patch Patch: The patch adds a new function 'end_ebml_master_crc32_preliminary' that preliminarily finishes the ebl element without destroying the buffer. The buffer can be used to update the ebml element later during mkv_write_trailer. But most important: mkv_write_header finishes with a valid mkv header again. --- libavformat/avio.h | 12 ++++++++++++ libavformat/aviobuf.c | 17 +++++++++++++++++ libavformat/matroskaenc.c | 34 +++++++++++++++++++++++++++++++--- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/libavformat/avio.h b/libavformat/avio.h index b1ce1d1..f2b9a6f 100644 --- a/libavformat/avio.h +++ b/libavformat/avio.h @@ -704,6 +704,18 @@ int avio_closep(AVIOContext **s); int avio_open_dyn_buf(AVIOContext **s); /** + * Return the written size and a pointer to the buffer. + * The AVIOContext stream is left intact. + * The buffer must NOT be freed. + * No padding is added to the buffer. + * + * @param s IO context + * @param pbuffer pointer to a byte buffer + * @return the length of the byte buffer + */ +int avio_get_dyn_buf(AVIOContext *s, uint8_t **pbuffer); + +/** * Return the written size and a pointer to the buffer. The buffer * must be freed with av_free(). * Padding of AV_INPUT_BUFFER_PADDING_SIZE is added to the buffer. diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index 134d627..bf7e5f8 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -1277,6 +1277,23 @@ int ffio_open_dyn_packet_buf(AVIOContext **s, int max_packet_size) return url_open_dyn_buf_internal(s, max_packet_size); } +int avio_get_dyn_buf(AVIOContext *s, uint8_t **pbuffer) +{ + DynBuffer *d; + + if (!s) { + *pbuffer = NULL; + return 0; + } + + avio_flush(s); + + d = s->opaque; + *pbuffer = d->buffer; + + return d->size; +} + int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer) { DynBuffer *d; diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 827d755..a2d8e44 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -367,6 +367,28 @@ static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, Matrosk *dyn_cp = NULL; } +/** +* Complete ebml master whithout destroying the buffer, allowing for later updates +*/ +static void end_ebml_master_crc32_preliminary(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv, + ebml_master master) +{ + uint8_t *buf, crc[4]; + int size, skip = 0; + + if (pb->seekable) { + + size = avio_get_dyn_buf(*dyn_cp, &buf); + if (mkv->write_crc && mkv->mode != MODE_WEBM) { + 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 + skip, size - skip); + end_ebml_master(pb, master); + } +} + static void put_xiph_size(AVIOContext *pb, int size) { ffio_fill(pb, 255, size / 255); @@ -1308,8 +1330,10 @@ static int mkv_write_tracks(AVFormatContext *s) return ret; } - if (pb->seekable && !mkv->is_live) + if (pb->seekable && !mkv->is_live) { put_ebml_void(pb, avio_tell(mkv->tracks_bc)); + end_ebml_master_crc32_preliminary(pb, &mkv->tracks_bc, mkv, mkv->tracks_master); + } else end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, mkv->tracks_master); @@ -1553,8 +1577,10 @@ static int mkv_write_tags(AVFormatContext *s) } if (mkv->tags.pos) { - if (s->pb->seekable && !mkv->is_live) + if (s->pb->seekable && !mkv->is_live) { put_ebml_void(s->pb, avio_tell(mkv->tags_bc)); + end_ebml_master_crc32_preliminary(s->pb, &mkv->tags_bc, mkv, mkv->tags); + } else end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, mkv->tags); } @@ -1810,8 +1836,10 @@ static int mkv_write_header(AVFormatContext *s) put_ebml_void(pb, 11); // assumes double-precision float to be written } } - if (s->pb->seekable && !mkv->is_live) + if (s->pb->seekable && !mkv->is_live) { put_ebml_void(s->pb, avio_tell(pb)); + end_ebml_master_crc32_preliminary(s->pb, &mkv->info_bc, mkv, mkv->info); + } else end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, mkv->info); pb = s->pb;