On Mon, Nov 16, 2015 at 05:28:58PM -0800, Chris Cunningham wrote: > To test this new patch, again use testcount.mp3 > <http://happyworm.com/jPlayerLab/halite/jumptest/testcount.mp3> (CBR, > corrupt TOC). > > Current ffmpeg (with none of my mp3 patches) > > ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 0 > > plays out "1395", which is correct > > > ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 1 > plays out "..378, 1389", which is incorrect due to the corrupted TOC. > > ./ffplay testcount.mp3 -ss 00:33:20 -fflags fastseek > also plays out "..378, 1389" because current fast_seek logic uses the TOC > whenever available for CBR and VBR alike. > > > After applying my *latest* patch: > > ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 0 > is unchanged: plays out "1395", which is correct > > ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 1 > is unchanged: still plays out "378, 1389". This is wrong, but it allows > users to force using TOC should they prefer. > > ./ffplay testcount.mp3 -ss 00:33:20 -fflags fastseek > *is changed: *plays out "1395", which is correct. fast seek no longer uses > the TOC for CBR files. > > >
> What do you think? I personally prefer this way so that usetoc is still > meaningful for CBR files. i tend to agree about usetocs meaningfullness but it seems breaking fate tests: --- ./tests/ref/seek/extra-mp3 2015-11-16 00:07:55.812389092 +0100 +++ tests/data/fate/seek-extra-mp3 2015-11-17 03:15:54.266446909 +0100 @@ -5,16 +5,14 @@ ret: 0 st: 0 flags:1 dts: 1.880816 pts: 1.880816 pos: 31544 size: 418 ret: 0 st: 0 flags:0 ts: 0.788334 ret: 0 st: 0 flags:1 dts: 0.809796 pts: 0.809796 pos: 14407 size: 418 -ret: 0 st: 0 flags:1 ts:-0.317499 -ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 1451 size: 440 +ret:-1 st: 0 flags:1 ts:-0.317499 ret: 0 st:-1 flags:0 ts: 2.576668 ret: 0 st: 0 flags:1 dts: 2.586122 pts: 2.586122 pos: 42828 size: 418 ret: 0 st:-1 flags:1 ts: 1.470835 ret: 0 st: 0 flags:1 dts: 1.462857 pts: 1.462857 pos: 24856 size: 418 ret: 0 st: 0 flags:0 ts: 0.365002 ret: 0 st: 0 flags:1 dts: 0.365714 pts: 0.365714 pos: 7302 size: 418 -ret: 0 st: 0 flags:1 ts:-0.740831 -ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 1451 size: 440 +ret:-1 st: 0 flags:1 ts:-0.740831 ret: 0 st:-1 flags:0 ts: 2.153336 ret: 0 st: 0 flags:1 dts: 2.168163 pts: 2.168163 pos: 36141 size: 418 ret: 0 st:-1 flags:1 ts: 1.047503 @@ -41,13 +39,11 @@ ret: 0 st: 0 flags:1 dts: 1.985306 pts: 1.985306 pos: 33215 size: 418 ret: 0 st:-1 flags:0 ts: 0.883340 ret: 0 st: 0 flags:1 dts: 0.888163 pts: 0.888163 pos: 15661 size: 418 -ret: 0 st:-1 flags:1 ts:-0.222493 -ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 1451 size: 440 +ret:-1 st:-1 flags:1 ts:-0.222493 ret: 0 st: 0 flags:0 ts: 2.671674 ret: 0 st: 0 flags:1 dts: 2.690612 pts: 2.690612 pos: 44500 size: 418 ret: 0 st: 0 flags:1 ts: 1.565841 -ret: 0 st: 0 flags:1 dts: 1.567347 pts: 1.567347 pos: 26528 size: 418 +ret: 0 st: 0 flags:1 dts: 1.541224 pts: 1.541224 pos: 26110 size: 418 ret: 0 st:-1 flags:0 ts: 0.460008 ret: 0 st: 0 flags:1 dts: 0.470204 pts: 0.470204 pos: 8974 size: 418 -ret: 0 st:-1 flags:1 ts:-0.645825 -ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 1451 size: 440 +ret:-1 st:-1 flags:1 ts:-0.645825 Test seek-extra-mp3 failed. Look at tests/data/fate/seek-extra-mp3.err for details. make: *** [fate-seek-extra-mp3] Error 1 make: *** Waiting for unfinished jobs.... > > Thanks, > Chris > > On Mon, Nov 16, 2015 at 4:58 PM, <chcunning...@chromium.org> wrote: > > > From: Chris Cunningham <chcunning...@chromium.org> > > > > "Fast seek" uses linear interpolation to find the position of the > > requested seek time. For CBR this is more direct than using the > > mp3 TOC and bypassing the TOC avoids problems when the TOC is > > corrupted (e.g. https://crbug.com/545914). > > > > For VBR, fast seek is not precise, so continue to prefer the TOC > > when available. > > --- > > libavformat/mp3dec.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > > index 32ca00c..e12266c 100644 > > --- a/libavformat/mp3dec.c > > +++ b/libavformat/mp3dec.c > > @@ -515,8 +515,10 @@ static int mp3_seek(AVFormatContext *s, int > > stream_index, int64_t timestamp, > > filesize = size - s->internal->data_offset; > > } > > > > - if ( (mp3->is_cbr || fast_seek) > > - && (mp3->usetoc == 0 || !mp3->xing_toc) > > + // When fast seeking, prefer to use the TOC when available for VBR > > files > > + // since av_rescale may not be accurate for VBR. For CBR, rescaling is > > + // always accurate and more direct than a TOC lookup. > > + if (fast_seek && (mp3->is_cbr || !mp3->xing_toc) > > && st->duration > 0 > > && filesize > 0) { > > ie = &ie1; > > -- > > 2.6.0.rc2.230.g3dd15c0 > > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
