From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id B977544A6D for ; Thu, 5 Jan 2023 21:58:39 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 90D3F68BDA0; Thu, 5 Jan 2023 23:58:36 +0200 (EET) Received: from mailrelay.ngus.net (mailrelay.ngus.net [109.237.26.52]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 63FA868BD42 for ; Thu, 5 Jan 2023 23:58:30 +0200 (EET) Received: from cpc102338-sgyl38-2-0-cust655.18-2.cable.virginm.net ([77.102.34.144] helo=zebop) by mailrelay.ngus.net with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1pDYFp-009yEf-Hd for ffmpeg-devel@ffmpeg.org; Thu, 05 Jan 2023 21:58:29 +0000 Received: from [192.168.101.3] by zebop with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.95) (envelope-from ) id 1pDYFn-004BE3-K1 for ffmpeg-devel@ffmpeg.org; Thu, 05 Jan 2023 21:58:29 +0000 Message-ID: Date: Thu, 5 Jan 2023 21:58:27 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20230105205842.1442824-1-tim@ngus.net> <20230105205842.1442824-2-tim@ngus.net> From: Tim Angus In-Reply-To: X-Spam-Score: -5.8 (-----) Subject: Re: [FFmpeg-devel] [PATCH 1/1] avformat/assenc: fix incorrect copy of null terminator X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 05/01/2023 21:11, Andreas Rheinhardt wrote: > Tim Angus: >> Signed-off-by: Tim Angus >> --- >> libavformat/assenc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/assenc.c b/libavformat/assenc.c >> index 1600f0a02b..07b6e3a171 100644 >> --- a/libavformat/assenc.c >> +++ b/libavformat/assenc.c >> @@ -69,7 +69,7 @@ static int write_header(AVFormatContext *s) >> ass->trailer = trailer; >> } >> >> - avio_write(s->pb, par->extradata, header_size); >> + avio_write(s->pb, par->extradata, header_size - 1); >> if (par->extradata[header_size - 1] != '\n') >> avio_write(s->pb, "\r\n", 2); >> ass->ssa_mode = !strstr(par->extradata, "\n[V4+ Styles]"); > 1. The rationale for the patch (that you mentioned in the cover letter) > should be part of the commit message. Fair enough. > 2. Did you run FATE with your patch? This should actually change the > output of some tests. Yes I did; no failures locally, though I see there are failures in this "patchwork" thingy, presumably that is running extra tests? > 3. The '\0' is not supposed to be accounted for in extradata_size; > extradata is supposed to be padded with AV_INPUT_BUFFER_PADDING_SIZE > zero bytes, the first of which also acts as trailing zero for formats > for which extradata is a C-string. (And anyway: There are cases where > header_size does not coincide with extradata_size, yet you are also > changing them.) > Having read this, I did a bit more digging and it appears as though the source of the extra nul may actually be embedded in the file I was having trouble with in the first place itself, so my patch is probably prematurely submitted, sorry. mkvtoolnix seems to extract the subtitle file with no trouble so it's not really clear where the fault lies. I'm probably better submitting a bug to the tracker rather than trying to fix it myself, tbh. (Out of interest, what is the policy WRT to broken files? You could make a reasonable case that the encoder should filter the extra nul, assuming of course that the extra nul is indeed extra and there is no ffmpeg bug.) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".