On Sat, Oct 17, 2015 at 04:18:14PM +0200, wm4 wrote: > On Sat, 17 Oct 2015 14:02:09 +0200 > Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > On Fri, Oct 16, 2015 at 08:04:08PM +0200, wm4 wrote: > > > Commit 2b3e9bbfb529e6bde238aeb511b55ebe461664c8 caused problems for a > > > certain API user: > > > > > > https://code.google.com/p/chromium/issues/detail?id=537725 > > > https://code.google.com/p/chromium/issues/detail?id=542032 > > > > > > The problem seems rather arbitrary, because if there's junk, anything > > > can happen. In this case, the imperfect junk skipping just caused it to > > > read different junk, from what I can see. > > > > > > We can improve the accuracy of junk detection by a lot by checking if 2 > > > consecutive frames use the same configuration. While in theory it might > > > be completely fine for the 1st frame to have a different format than the > > > 2nd frame, it's exceedingly unlikely, and I can't think of a legitimate > > > use-case. > > > > > > This is approximately the same mpg123 does for junk skipping, and the > > > set of compared header bits is exactly the same. > > > --- > > > The reason Chromium had so much problems with this is that they don't > > > allow mid-stream format changes at all, and the junk triggered format > > > changes. > > > > > > (Would be nice if Google paid me for this.) > > > --- > > > libavformat/mp3dec.c | 25 ++++++++++++++++++------- > > > 1 file changed, 18 insertions(+), 7 deletions(-) > > > > > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > > > index d3080d7..e7b6234 100644 > > > --- a/libavformat/mp3dec.c > > > +++ b/libavformat/mp3dec.c > > > @@ -54,7 +54,7 @@ typedef struct { > > > int is_cbr; > > > } MP3DecContext; > > > > > > -static int check(AVIOContext *pb, int64_t pos); > > > +static int check(AVIOContext *pb, int64_t pos, uint32_t *header); > > > > > > /* mp3 read */ > > > > > > @@ -374,12 +374,21 @@ static int mp3_read_header(AVFormatContext *s) > > > > > > off = avio_tell(s->pb); > > > for (i = 0; i < 64 * 1024; i++) { > > > + uint32_t header, header2; > > > + int frame_size; > > > if (!(i&1023)) > > > ffio_ensure_seekback(s->pb, i + 1024 + 4); > > > - if (check(s->pb, off + i) >= 0) { > > > - av_log(s, AV_LOG_INFO, "Skipping %d bytes of junk at > > > %"PRId64".\n", i, off); > > > - avio_seek(s->pb, off + i, SEEK_SET); > > > - break; > > > + frame_size = check(s->pb, off + i, &header); > > > + if (frame_size > 0) { > > > + avio_seek(s->pb, off, SEEK_SET); > > > + ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4); > > > + if (check(s->pb, off + i + frame_size, &header2) >= 0 && > > > + (header & 0xFFFEFCF0) == (header2 & 0xFFFEFCF0)) > > > > This also requires the bitrate and stereo type to match > > teh bitrate can change in VBR, the stereo coding type can also change > > i think > > > > [...] > > What do you suggest?
does checking only the remaining parts/bits work ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
