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-9N3ejNkMW9yU0k4ZFE/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 ... > 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. Adding a flag that "must be parsed" would be a incompatible API change. It would require a major version bump also this patchset changes streamcopy try: ./ffmpeg-ref -i matrixbench_mpeg2.mpg -acodec mp3 -vn -t 1 test-ref.mov ./ffmpeg -i matrixbench_mpeg2.mpg -acodec mp3 -vn -t 1 test.mov ./ffmpeg-ref -i test-ref.mov -acodec copy test2-ref.mov ./ffmpeg -i test.mov -acodec copy test2.mov old code: ./ffmpeg-ref -i test-ref.mov -aframes 3 -f framecrc - 0, 0, 0, 1152, 4608, 0xa2a00df2 0, 1152, 1152, 1152, 4608, 0xa573dfd4 0, 2304, 2304, 1152, 4608, 0x8994a906 and ./ffmpeg-ref -i test2-ref.mov -aframes 3 -f framecrc - 0, 0, 0, 1152, 4608, 0xa2a00df2 0, 1152, 1152, 1152, 4608, 0xa573dfd4 0, 2304, 2304, 1152, 4608, 0x8994a906 new code: ./ffmpeg -i test.mov -aframes 3 -f framecrc 0, 0, 0, 1152, 4608, 0xa573dfd4 0, 1152, 1152, 1152, 4608, 0x8994a906 0, 2304, 2304, 1152, 4608, 0x824d1a30 and ./ffmpeg -i test2.mov -aframes 3 -f framecrc - 0, 0, 0, 1152, 4608, 0xa2a00df2 0, 1, 1, 1152, 4608, 0xa573dfd4 0, 1152, 1152, 1152, 4608, 0x8994a906 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
