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 E255040B5E for ; Mon, 27 Dec 2021 20:04:21 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 14E1968B046; Mon, 27 Dec 2021 22:04:19 +0200 (EET) Received: from mout-p-201.mailbox.org (mout-p-201.mailbox.org [80.241.56.171]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 85C4668AFDD for ; Mon, 27 Dec 2021 22:04:10 +0200 (EET) Received: from smtp102.mailbox.org (smtp102.mailbox.org [IPv6:2001:67c:2050:105:465:1:3:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-201.mailbox.org (Postfix) with ESMTPS id 4JN7sk22KhzQk9Y for ; Mon, 27 Dec 2021 21:04:10 +0100 (CET) X-Virus-Scanned: amavisd-new at heinlein-support.de Message-ID: <1df7ec4a-45cd-a6b7-2096-afaa19d09d08@gyani.pro> Date: Tue, 28 Dec 2021 01:33:54 +0530 MIME-Version: 1.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20211227055711.63060-1-ffmpeg@gyani.pro> <20211227190841.GW2829255@pb2> From: Gyan Doshi In-Reply-To: <20211227190841.GW2829255@pb2> Subject: Re: [FFmpeg-devel] [PATCH] avformat/mov: correct 0 valued entries in stts 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 2021-12-28 12:38 am, Michael Niedermayer wrote: > On Mon, Dec 27, 2021 at 11:27:10AM +0530, Gyan Doshi wrote: >> As per ISO 14496-12, sample duration of 0 is invalid except for >> the last entry. >> >> In addition, also catch 0 value for sample count. >> --- >> libavformat/mov.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index 2aed6e80ef..fb7406cdd6 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -2968,6 +2968,18 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >> av_log(c->fc, AV_LOG_TRACE, "sample_count=%d, sample_duration=%d\n", >> sample_count, sample_duration); >> >> + if (!sample_count) { >> + av_log(c->fc, AV_LOG_WARNING, "invalid sample count of 0 in stts for st %d at entry %u; changing to 1.\n", >> + c->fc->nb_streams-1, i); >> + sc->stts_data[i].count = sample_count = 1; >> + } >> + >> + if (!sample_duration && i != entries-1) { >> + av_log(c->fc, AV_LOG_WARNING, "invalid sample delta of 0 in stts for st %d at entry %u; changing to 1.\n", >> + c->fc->nb_streams-1, i); >> + sc->stts_data[i].duration = sample_duration = 1; >> + } >> + >> duration+=(int64_t)sample_duration*(uint64_t)sample_count; >> total_sample_count+=sample_count; > This does not produce the same output > tickets/2096/m.f4v > > videos/stretch.mov (2344 matches for "invalid" after this patch) > > tickets/976/CodecCopyFailing.mp4 > > But there are many more, some maybe even generated by FFmpeg Where do I find these files? > Taking a step back, the problem started with > 203b0e3561dea1ec459be226d805abe73e7535e5 > which broke a real world file which was outside the specification Just to clarify, it did not break that file. That file uses stts in an unusual way. Before 2015, lavf exported packets with the same desync as the other demuxers do so till today. Andreas' patch added a hack to make it play in sync. My patch 203b0e356 broke that hack. The patch for max_stts_delta is a way to restore it back. > you then suggested a fix which crashed with some fuzzed files which > where outside the specification > > and now this fix on top which changes real world files which > are outside the specification > > I think, maybe you should consider the "outside the specification" > more. The code above directly and intentionally changes values. > So as a reviewer i have to ask the obvious, is that change a > bugfix or a bug ? Not surprising that the output of out-of-spec files is different - that's expected, intended and trivial. It would be a bug if in-spec files were treated differently. FATE passes. If there's a specific / "correct" playback for these files like sync issues, I'll see if I can restore it. Regards, Gyan _______________________________________________ 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".