On Fri, Sep 02, 2016 at 01:00:59PM -0700, Sasi Inguva wrote: > On Aug 31, 2016 5:23 AM, "Michael Niedermayer" <mich...@niedermayer.cc> > wrote: > > > > On Tue, Aug 30, 2016 at 06:37:22PM -0700, Sasi Inguva wrote: > > > On Sun, Aug 28, 2016 at 3:10 AM, Michael Niedermayer > <mich...@niedermayer.cc > > > > 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 > > > > <mich...@niedermayer.cc > > > > > > 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 > > > <https://drive.google.com/file/d/0Bz6XfEJZ-9N3ejNkMW9yU0k4ZF > E/view?usp=sharing> > > > 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. > > > > My first question is, entirely independant of the implementation from > > the patches. What is the correct output ? (my primary focus is on > > the timestamps) > > > > > > Also if there are discontinuities, AVFMT_TS_DISCONT is normally set > > (and this never happens for files with indexes but only for files > > that dont have indexes like mpeg*) > > players will generally seek by file position if AVFMT_TS_DISCONT is > > set (because timestamps are ambigous in that case) > > > > iam not sure how to seek reliably by file position in a mov > > with edit lists and stll have the file positions actually be file > > positions of the frames, so this direction gets tricky too ... > > > I'll try do describe the behavior of the timestamps and seek after all > these patches are applied. > Reading: > i) Decoding (using av_codec_video* API) - Non-repeated monotonically > increasing timestamps.
> ii) Just demuxing - Repeated timestamps in case of Single edit list . can you explain why you insist on this ? why do you assign invalid timestamps to discarded packets ? These packets are just normal packets in the current API, applications will treat them like any other packet and the timestamps would likely cause problems, confusion and bugreports > Non-monotonic timestamps in case of multiple edit lists. can you elaborate on how exactly they are non monotonic you said above that they are monotonic after the decoder, i would have expected that they would still be non monotonic after the decoder for multiple edit lists so maybe i misunderstood [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Breaking DRM is a little like attempting to break through a door even though the window is wide open and the only thing in the house is a bunch of things you dont want and which you would get tomorrow for free anyway
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel