| Message ID | 20160914205729.GS4975@nb4 |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, Sep 15, 2016 at 01:08:40PM -0700, Sasi Inguva wrote: > On Wed, Sep 14, 2016 at 1:57 PM, Michael Niedermayer <michael@niedermayer.cc > > wrote: > > > On Wed, Sep 14, 2016 at 12:20:52AM -0700, Sasi Inguva wrote: > > > On Tue, Sep 13, 2016 at 4:39 PM, Sasi Inguva <isasi@google.com> wrote: > > > > > > > Sorry for the very late reply. I was busy with other things. > > > > > > > > On Sat, Sep 3, 2016 at 4:48 PM, Michael Niedermayer < > > > > michael@niedermayer.cc> wrote: > > > > > > > >> On Sat, Sep 03, 2016 at 12:06:39PM -0700, Sasi Inguva wrote: > > > >> > Hi Michael, > > > >> > > > > >> > In fact from audacity I see that out-ingu.wav out-mp3.wav are almost > > > >> > equivalent, > > > >> > > > >> They do not match. (and that is alot more vissible if you scale the > > > >> time axis up a bit) > > > >> > > > >> You also dont use the existing API for handling initial padding/skip > > > >> And you didnt reply to this concern > > > >> its probably not that hard to fix that ... > > > >> instead of droping just at packet granularity you would set the stuff > > > >> for droping at sample granularity (too) > > > >> > > > > > > > > Yes. Looking at it more closely now, they don't matrch exactly and > > this is > > > > because as you said, number of samples to drop is not exactly multiple > > of > > > > number of packets. In that case we need to partially discard some > > samples > > > > of a packet. This can be done by using AV_PKT_SIDE_DATA_SKIP_SAMPLES. > > I > > > > can change the code to use this packet side data instead of discard > > packet > > > > flag, if it is ok. > > > > ok > > > > I have put my foot in my mouth here. There is no way I can add > AV_PKT_SIDE_DATA_SKIP_SAMPLES to each packet, unless I add a new field > "skip_samples" in AVIndexEntry and populate it in mov_fix_index function. > Adding another entry to AVIndexEntry seems contentious because we want to > keep the AVIndexEntry as compact as possible, I presume. So what I wil do > for now is just set st->skip_samples field in mov_fix_index function for > the first audio edit list. So at least single edit list audio cases ,will > be correct. In multiple edit list case, the the first edit will be > correctly output but the second audio edit might have some samples extra. > Also note that, there is no mechanism for doing "discard_padding" too, in > MOV demuxer. If we have skip_samples and discard_padding fields in > AVIndexEntry, then we can implement both of them correctly. > > With the attached patch the test that you have showed, comparing > out-ingu.wav and out-mp3.wav - are exactly equivalent. yes, the patch also improves some audio sync cases that where bad but it breaks audio sync with faac ./ffmpeg-ref -f u8 -ar 16000 -ac 1 -i sync_audio.raw -acodec libfaac test-ref.mov ./ffmpeg_g -f u8 -ar 16000 -ac 1 -i sync_audio.raw -acodec libfaac test.mov ./ffmpeg-ref -i test-ref.mov test-ref.mov.wav ./ffmpeg_g -i test.mov test.mov.wav [...] > @@ -2756,6 +2757,343 @@ static int mov_read_sbgp(MOVContext *c, AVIOContext *pb, MOVAtom atom) > return pb->eof_reached ? AVERROR_EOF : 0; > } > > +/** > + * Get ith edit list entry (media time, duration). > + */ > +static int get_edit_list_entry(const MOVStreamContext *msc, > + unsigned int edit_list_index, > + int64_t *edit_list_media_time, > + int64_t *edit_list_duration, > + int64_t global_timescale) > +{ > + if (edit_list_index == msc->elst_count) { > + return 0; > + } > + *edit_list_media_time = msc->elst_data[edit_list_index].time; > + *edit_list_duration = msc->elst_data[edit_list_index].duration; > + /* duration is in global timescale units;convert to msc timescale */ > + *edit_list_duration = av_rescale(*edit_list_duration, msc->time_scale, > + global_timescale); global_timescale can be 0 here leading to division by 0 [...]
--- ./tests/ref/fate/copy-trac236 2016-09-14 15:56:10.845886847 +0200 +++ tests/data/fate/copy-trac236 2016-09-14 22:44:44.494403284 +0200 @@ -1,5 +1,5 @@ -9b95afdb39b426a33bc962889f820aed *tests/data/fate/copy-trac236.mov -630802 tests/data/fate/copy-trac236.mov +d6e3d97b522ce881ed29c5da74cc7e63 *tests/data/fate/copy-trac236.mov +630810 tests/data/fate/copy-trac236.mov #tb 0: 100/2997 #media_type 0: video #codec_id 0: rawvideo