[FFmpeg-cvslog] avformat/mp3dec: improve junk skipping heuristic

[wm4]

ffmpeg | branch: master | wm4 <nfxjfg at googlemail.com> | Tue Oct 20 12:17:21 2015 +0200| [de1b1a7da9e6ddf42447271e519099a88b389e4a] | committer: wm4

avformat/mp3dec: improve junk skipping heuristic

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. The
set of compared header bits is the same as the libavcodec mp3 parser
uses for similar purposes.

> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=de1b1a7da9e6ddf42447271e519099a88b389e4a
---

 libavformat/mp3dec.c |   28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index d3080d7..32ca00c 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -42,6 +42,9 @@
 
 #define XING_TOC_COUNT 100
 
+#define SAME_HEADER_MASK \
+   (0xffe00000 | (3 << 17) | (3 << 10) | (3 << 19))
+
 typedef struct {
     AVClass *class;
     int64_t filesize;
@@ -54,7 +57,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 +377,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 & SAME_HEADER_MASK) == (header2 & SAME_HEADER_MASK))
+            {
+                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;
+            }
         }
         avio_seek(s->pb, off, SEEK_SET);
     }
@@ -420,7 +432,7 @@ static int mp3_read_packet(AVFormatContext *s, AVPacket *pkt)
 
 #define SEEK_WINDOW 4096
 
-static int check(AVIOContext *pb, int64_t pos)
+static int check(AVIOContext *pb, int64_t pos, uint32_t *ret_header)
 {
     int64_t ret = avio_seek(pb, pos, SEEK_SET);
     unsigned header;
@@ -434,6 +446,8 @@ static int check(AVIOContext *pb, int64_t pos)
     if (avpriv_mpegaudio_decode_header(&sd, header) == 1)
         return -1;
 
+    if (ret_header)
+        *ret_header = header;
     return sd.frame_size;
 }
 
@@ -461,7 +475,7 @@ static int64_t mp3_sync(AVFormatContext *s, int64_t target_pos, int flags)
             continue;
 
         for(j=0; j<MIN_VALID; j++) {
-            ret = check(s->pb, pos);
+            ret = check(s->pb, pos, NULL);
             if(ret < 0)
                 break;
             if ((target_pos - pos)*dir <= 0 && abs(MIN_VALID/2-j) < score) {