From patchwork Wed Aug 31 01:37:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sasi Inguva X-Patchwork-Id: 346 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.140.134 with SMTP id o128csp141642vsd; Tue, 30 Aug 2016 18:42:54 -0700 (PDT) X-Received: by 10.28.45.65 with SMTP id t62mr18104635wmt.14.1472607773945; Tue, 30 Aug 2016 18:42:53 -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 ea8si41428527wjb.92.2016.08.30.18.42.53; Tue, 30 Aug 2016 18:42:53 -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=@google.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 Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8A432689D79; Wed, 31 Aug 2016 04:42:43 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ua0-f171.google.com (mail-ua0-f171.google.com [209.85.217.171]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D9F8F689D71 for ; Wed, 31 Aug 2016 04:42:35 +0300 (EEST) Received: by mail-ua0-f171.google.com with SMTP id l94so64232052ual.0 for ; Tue, 30 Aug 2016 18:42:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=rBBIsTU5LXYi+4yv3JzGde0BmY/bKIDbJwT/avmgl5I=; b=QYqIzU9e5xmG0+KyVkEuXyG8WJY4CObegGOww2+SwyFXK/aYG3aMDbnQvxA4160mfq hbxe9pXjb0/rwoeawBCYN2blwcGpslAtdsD0cxtjUSmvG9V48b97i+eGN7Ax6k4NRypI RG6VvVj4ufq3fSoFco6/WuD4DRJOkwU5ZjTdlhAik3GgWXsJrmqdad9NgOtld4TyZbdV UGDrjg0BSkNDYDXjGDk8o4h0VcwiKrO2m7RRLFitGFwuZ2Q/ekX7brDYAoYDDLTrOEHa xYXwKtYxSJS1IM2VTZn2VG11elyyi/rj2minfA71C10vdTI1OEzY/Br4umEoJ5V0AEjK vY+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=rBBIsTU5LXYi+4yv3JzGde0BmY/bKIDbJwT/avmgl5I=; b=PSlaaIMOGxomQdE1xkvZhO4Vf5qu2ThwKtdNFHZv7uVPqffRj68NtaokTcxsHVQ8yb CLftpqsDRDCbIEDWFeX2Ot7Wje3vCP/958uX1VI8Eh4ok1d9W7Iao78rsoB/XQnDZ/PJ rt7mm7OB8G8Popr7s3XgpVWNEhtfp1swZFbwP8EQmI7wawqgJJalxdKnftFebLy7nrVO GWv4uZ6Rb9nSaSC9UVWqdbotZKHW88jO70vzJ03UmE3JNorwdRAmMLoK+QePlBW7gk51 lzv9prTEWBvsuQ/6HLTJSq2X8MBWRkYJU9VSvrr1nBP3RNc0WuYL36gGHXy+H5nVwzMC reZw== X-Gm-Message-State: AE9vXwONyn5+LfUU69XWi7TaR8QhOqeoTbfxLJO0gxZKG+wMcw9PvDHMymHJ8CYoTqZ1Yvh0MpQhe2+n5V1Jtlmq X-Received: by 10.176.80.199 with SMTP id d7mr3546820uaa.139.1472607445071; Tue, 30 Aug 2016 18:37:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.31.153.207 with HTTP; Tue, 30 Aug 2016 18:37:22 -0700 (PDT) In-Reply-To: <20160828101015.GY5460@nb4> References: <1471313097-26998-1-git-send-email-isasi@google.com> <1471313097-26998-2-git-send-email-isasi@google.com> <20160824230109.GP5460@nb4> <20160825023726.GR5460@nb4> <20160825220105.GF5460@nb4> <20160827005539.GM5460@nb4> <20160828101015.GY5460@nb4> From: Sasi Inguva Date: Tue, 30 Aug 2016 18:37:22 -0700 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH 4/4] lavf/mov: Add support for edit list parsing. 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" On Sun, Aug 28, 2016 at 3:10 AM, Michael Niedermayer wrote: > On Sat, Aug 27, 2016 at 03:30:24PM -0700, Sasi Inguva wrote: > > On Fri, Aug 26, 2016 at 5:55 PM, Michael Niedermayer > > > wrote: > > > > > On Fri, Aug 26, 2016 at 12:49:19PM -0700, Sasi Inguva wrote: > > > > I think there is some bug in mp3 decoder which is making skip > > > > samples -1431655766 for ~/tickets/5528/fire.mp3 . For now I have > removed > > > > the assert from the 3rd commit. > > > > For the file one.mov , I think the audio has edit list with start > time > > > > correspending to the second sample - (which should be media time > 1024 if > > > I > > > > guess correctly). This indicates that the first sample be used for > > > encoder > > > > delay. > > > > Previously without edit list parsing , we used to offset the > start_dts > > > by > > > > -1024 to make the second sample timestamp start from zero. > > > > sc->time_offset = start_time - empty_duration; > > > > - current_dts = -sc->time_offset; > > > > if (sc->ctts_count>0 && sc->stts_count>0 && > > > > > > > > But now edit list parsing handles the rebasing of timestamps to zero, > > > > because it will assign increasing timestamps starting from zero, to > > > > samples present in the edit list. > > > > > > > Because the first sample is not in the > > > > edit list, we mark it as DISCARD, which flag av_decode_audio4 will > look > > > at > > > > and decode-and-discard that frame. So it wouldn't matter what the > first > > > > sample timestamp should be assigned because it is anyway discarded > after > > > > decode. > > > > > > current applications using libavformat have no knowledge of the > > > discard flag you can add the DISCARD flag, you can set it on packets > but > > > applications written or built for libavformat 57 dont have to know > > > this flag and can treat the packets like any other packet. > > > > > > > Yes. they can treat the packet like any other packet. They don't have to > > know about the discard flag. > > > > Adding this feature without a major version bump requires things to > > > still work reasonable with the old semantics that apps are build > > > around. That should be possible unless iam missing something > > > > > > As I have pointed out earlier this code will change the timestamps of the > > packets. In the case of multiple edit lists, the code will also repeat > some > > packets, and might make the timestamps non-monotonous. I don't know if > the > > last behavior is not an acceptable expectation from av_read_frame. > > if timestamps repeat then it will not be possible to seek in the file > by timestamp (to all positions) and i suspect also not by byte position. > How would one seek then ? > or do i misunderstand ? > > In case of MOV container, the seek is done using av_index_search_timestamp function http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/mov.c;h=f4999068519f1f06f6b5d84ca007148e74e5a82e;hb=HEAD#l5419 . i) In case of single edit list , the timestamps will only be repeated but not non-monotonous. In that case av_index_search_timestamp will still work correctly, only that it will seek to any one of the packets with the same timestamp. However when we decode the file, then all of the discarded packets with similar timestamps should be discarded and only the non-discarded packet will bet output. So in short, ./ffprobe -read_intervals 0.0 -show_frames -print_format compact one-editlist-audio.mov should give exactly the same behavior as before while -show_packets will show more discarded packets at the start. I had to change the patch (4) a bit to make the audio-seek on MOV to work correctly, so please reapply the attached patch to test. ii) In case of multiple edit lists , timestamps might be non monotonous so existing av_index_search_timestamp implementation won't be correct, since it assumes sorted timestamps. However making it work for discarded packets is not that hard. I have attached a 5th patch that changes av_index_search function. This fixes the issue in (i) also > > However as I've pointed out, we are already showing the wrong packets for > > videos with multiple edit lists by not parsing the edit lists currently, > > which will introduce AVSync issues. So this patch wont make things any > > worse for those cases in my view. > > Is it difficult to adjust the timestamps of discarded packets to avoid > timestamp discontinuities ? (for the cases where this is possible of > course only) > the timestamps for discarded packets i would assume are meaningless in > the new semenatics but they matter for the old semantics > again, please correct me if iam wrong > > The way fix_index is implemented it is difficult to change the timestamps to avoid discotinuities and still have the timeline the same as MOV edit lists would intend. The timestamps for discarded packets are meaningless to av_decode_* functions because they parse the DISCARD flag and ignore the packets. I am not sure, what you mean by semantics though, because I don't think I am changing any user expectations defined by the mov_read_frame mov_seek_frame functions . If by semantics, you mean that user expects to see monotonically increasing timestamps for the "demuxed " packets then yes that expectation changes to " monotonically increasing timestamps for the "demuxed and non-discarded" packets" and user has to parse the discard flag. And to reiterate , this semantic is only affected in the case of files with multiple edit lists which are very less in number , since in case of single edit list videos timestamps will only be repeated and still are monotonically increasing. As of now with 5th patch the seek also works with edit lists. I can hide this functionality under a flag like "support_edit_liist" in MOV demuxer, and turn it off by default, if u think that is better. > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > If you drop bombs on a foreign country and kill a hundred thousand > innocent people, expect your government to call the consequence > "unprovoked inhuman terrorist attacks" and use it to justify dropping > more bombs and killing more people. The technology changed, the idea is > old. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > From f2a7a9d85728a1fa7d4afdc280a22aaaa4d91a8e Mon Sep 17 00:00:00 2001 From: Sasi Inguva Date: Tue, 30 Aug 2016 17:57:05 -0700 Subject: [PATCH 5/5] lavf/utils: Support av_index_search_timestamp in case of AVIndexEntry with discarded packets. Signed-off-by: Sasi Inguva --- libavformat/utils.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libavformat/utils.c b/libavformat/utils.c index d7f3c7a..f26ce58 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -1934,6 +1934,16 @@ int ff_index_search_timestamp(const AVIndexEntry *entries, int nb_entries, while (b - a > 1) { m = (a + b) >> 1; + + // Search for the next non-discarded packet. + while ((entries[m].flags & AVINDEX_DISCARD_FRAME) && m < b) { + m++; + if (m == b && entries[m].timestamp >= wanted_timestamp) { + m = b - 1; + break; + } + } + timestamp = entries[m].timestamp; if (timestamp >= wanted_timestamp) b = m; -- 2.8.0.rc3.226.g39d4020