[FFmpeg-devel] [PATCH] vp9_parser: set profile in AVCodecContext

[chcunningham]

---
 libavcodec/vp9_parser.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c
index 9531f34a32..b6b621198b 100644
--- a/libavcodec/vp9_parser.c
+++ b/libavcodec/vp9_parser.c
@@ -43,6 +43,8 @@ static int parse(AVCodecParserContext *ctx,
 profile |= get_bits1(&gb) << 1;
 if (profile == 3) profile += get_bits1(&gb);
 
+avctx->profile = profile;
+
 if (get_bits1(&gb)) {
 keyframe = 0;
 } else {
-- 
2.19.1.568.g152ad8e336-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak

[chcunningham]

Chromium fuzzing produced a whacky file with extra tkhds. This caused
an AVStream that was already in use to be corrupted by assigning it a
new id, which blows up later in mov_read_trun because the
MOVFragmentStreamInfo.index_entry now points OOB.
---
 libavformat/isom.h | 3 ++-
 libavformat/mov.c  | 7 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index e629663949..e14d670f2f 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -230,7 +230,8 @@ typedef struct MOVStreamContext {
 
 uint32_t format;
 
-int has_sidx;  // If there is an sidx entry for this stream.
+int has_sidx;  ///< If there is a sidx entry for this stream.
+int has_tkhd;  ///< If there is a tkhd entry for this stream.
 struct {
 struct AVAESCTR* aes_ctr;
 unsigned int per_sample_iv_size;  // Either 0, 8, or 16.
diff --git a/libavformat/mov.c b/libavformat/mov.c
index ec57a05803..47c53d7992 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4438,6 +4438,12 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 st = c->fc->streams[c->fc->nb_streams-1];
 sc = st->priv_data;
 
+// Each stream (trak) should have exactly 1 tkhd. This catches bad files 
and
+// avoids corrupting AVStreams mapped to an earlier tkhd.
+if (sc->has_tkhd)
+return AVERROR_INVALIDDATA;
+sc->has_tkhd = 1;
+
 version = avio_r8(pb);
 flags = avio_rb24(pb);
 st->disposition |= (flags & MOV_TKHD_FLAG_ENABLED) ? 
AV_DISPOSITION_DEFAULT : 0;
@@ -4704,6 +4710,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 break;
 }
 }
+av_assert0(index_entry_pos <= st->nb_index_entries);
 
 avio_r8(pb); /* version */
 flags = avio_rb24(pb);
-- 
2.20.0.rc2.403.gdbc3b29805-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavf/id3v2: fail read_apic on EOF reading mimetype

[chcunningham]

avio_read may return EOF, leaving the mimetype array unitialized. fail
early when this occurs to avoid using the array in an unitialized state.
---
 libavformat/id3v2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index f7de26a1d8..7c4d1f8677 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -612,7 +612,9 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, 
int taglen,
 if (isv34) {
 taglen -= avio_get_str(pb, taglen, mimetype, sizeof(mimetype));
 } else {
-avio_read(pb, mimetype, 3);
+if (avio_read(pb, mimetype, 3) < 0)
+goto fail;
+
 mimetype[3] = 0;
 taglen-= 3;
 }
-- 
2.20.0.rc2.403.gdbc3b29805-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavf/id3v2: fail read_apic on EOF reading mimetype

[chcunningham]

avio_read may return EOF, leaving the mimetype array unitialized. fail
early when this occurs to avoid using the array in an unitialized state.
---
 libavformat/id3v2.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index f7de26a1d8..f06b9a787e 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -609,10 +609,13 @@ static void read_apic(AVFormatContext *s, AVIOContext 
*pb, int taglen,
 taglen--;
 
 /* mimetype */
+memset(mimetype, 0, sizeof(mimetype));
 if (isv34) {
 taglen -= avio_get_str(pb, taglen, mimetype, sizeof(mimetype));
 } else {
-avio_read(pb, mimetype, 3);
+if (avio_read(pb, mimetype, 3) < 0)
+goto fail;
+
 mimetype[3] = 0;
 taglen-= 3;
 }
-- 
2.20.0.rc2.403.gdbc3b29805-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak

[chcunningham]

Chromium fuzzing produced a whacky file with extra tkhds. This caused
an AVStream that was already in use to be corrupted by assigning it a
new id, which blows up later in mov_read_trun because the
MOVFragmentStreamInfo.index_entry now points OOB.
---
 libavformat/mov.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index ec57a05803..6f92742e23 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1326,6 +1326,10 @@ static int update_frag_index(MOVContext *c, int64_t 
offset)
 return -1;
 
 for (i = 0; i < c->fc->nb_streams; i++) {
+// Avoid building frag index if streams lack track id.
+if (c->fc->streams[i]->id < 0)
+return AVERROR_INVALIDDATA;
+
 frag_stream_info[i].id = c->fc->streams[i]->id;
 frag_stream_info[i].sidx_pts = AV_NOPTS_VALUE;
 frag_stream_info[i].tfdt_dts = AV_NOPTS_VALUE;
@@ -4154,7 +4158,7 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 
 st = avformat_new_stream(c->fc, NULL);
 if (!st) return AVERROR(ENOMEM);
-st->id = c->fc->nb_streams;
+st->id = -1;
 sc = av_mallocz(sizeof(MOVStreamContext));
 if (!sc) return AVERROR(ENOMEM);
 
@@ -4438,6 +4442,11 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 st = c->fc->streams[c->fc->nb_streams-1];
 sc = st->priv_data;
 
+// Each stream (trak) should have exactly 1 tkhd. This catches bad files 
and
+// avoids corrupting AVStreams mapped to an earlier tkhd.
+if (st->id != -1)
+return AVERROR_INVALIDDATA;
+
 version = avio_r8(pb);
 flags = avio_rb24(pb);
 st->disposition |= (flags & MOV_TKHD_FLAG_ENABLED) ? 
AV_DISPOSITION_DEFAULT : 0;
@@ -4704,6 +4713,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 break;
 }
 }
+av_assert0(index_entry_pos <= st->nb_index_entries);
 
 avio_r8(pb); /* version */
 flags = avio_rb24(pb);
-- 
2.20.0.rc2.403.gdbc3b29805-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavf/id3v2: fail read_apic on EOF reading mimetype

[chcunningham]

avio_read may return EOF, leaving the mimetype array unitialized. fail
early when this occurs to avoid using the array in an unitialized state.
---
 libavformat/id3v2.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index f7de26a1d8..5fe055b591 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -590,7 +590,7 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, 
int taglen,
   int isv34)
 {
 int enc, pic_type;
-char mimetype[64];
+char mimetype[64] = {0};
 const CodecMime *mime = ff_id3v2_mime_tags;
 enum AVCodecID id = AV_CODEC_ID_NONE;
 ID3v2ExtraMetaAPIC *apic  = NULL;
@@ -612,7 +612,9 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, 
int taglen,
 if (isv34) {
 taglen -= avio_get_str(pb, taglen, mimetype, sizeof(mimetype));
 } else {
-avio_read(pb, mimetype, 3);
+if (avio_read(pb, mimetype, 3) < 0)
+goto fail;
+
 mimetype[3] = 0;
 taglen-= 3;
 }
-- 
2.20.0.405.gbc1bbc6f85-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

[chcunningham]

Return replaces an assert0. libfuzzer generated a testcase that
triggered this assert (codec=0), causing a crash of chrome's renderer.
---
 libavcodec/gsm_parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/gsm_parser.c b/libavcodec/gsm_parser.c
index 1054a30ca9..5cf2235f73 100644
--- a/libavcodec/gsm_parser.c
+++ b/libavcodec/gsm_parser.c
@@ -56,7 +56,7 @@ static int gsm_parse(AVCodecParserContext *s1, AVCodecContext 
*avctx,
 s->duration   = GSM_FRAME_SIZE * 2;
 break;
 default:
-av_assert0(0);
+return -1;
 }
 }
 
-- 
2.20.1.495.gaa96b0ce6b-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mov: validate chunk_count vs stsc_data

[chcunningham]

Bad content may contain stsc boxes with a first_chunk index that
exceeds stco.entries (chunk_count).

mov_get_stsc_samples now checks for this and returns 0 when
values are invalid.

Also updates MOVStsc to use unsigned ints, per spec.
---
 libavformat/isom.h | 6 +++---
 libavformat/mov.c  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index e629663949..8e0d8355b3 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -59,9 +59,9 @@ typedef struct MOVStts {
 } MOVStts;
 
 typedef struct MOVStsc {
-int first;
-int count;
-int id;
+unsigned int first;
+unsigned int count;
+unsigned int id;
 } MOVStsc;
 
 typedef struct MOVElst {
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..dcf4ee8dc1 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2690,11 +2690,11 @@ static inline int mov_stsc_index_valid(unsigned int 
index, unsigned int count)
 /* Compute the samples value for the stsc entry at the given index. */
 static inline int64_t mov_get_stsc_samples(MOVStreamContext *sc, unsigned int 
index)
 {
-int chunk_count;
+unsigned int chunk_count = 0;
 
 if (mov_stsc_index_valid(index, sc->stsc_count))
 chunk_count = sc->stsc_data[index + 1].first - 
sc->stsc_data[index].first;
-else
+else if (sc->chunk_count >= sc->stsc_data[index].first)
 chunk_count = sc->chunk_count - (sc->stsc_data[index].first - 1);
 
 return sc->stsc_data[index].count * (int64_t)chunk_count;
-- 
2.20.1.611.gfbb209baf1-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/isom.h: use usnigned types in MOVStsc

[chcunningham]

Unsigned types match the isobmff spec.
---
 libavformat/isom.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index e629663949..8e0d8355b3 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -59,9 +59,9 @@ typedef struct MOVStts {
 } MOVStts;
 
 typedef struct MOVStsc {
-int first;
-int count;
-int id;
+unsigned int first;
+unsigned int count;
+unsigned int id;
 } MOVStsc;
 
 typedef struct MOVElst {
-- 
2.20.1.611.gfbb209baf1-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mov: validate chunk_count vs stsc_data

[chcunningham]

Bad content may contain stsc boxes with a first_chunk index that
exceeds stco.entries (chunk_count).

mov_get_stsc_samples now checks for this and returns 0 when
values are invalid.
---
 libavformat/mov.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..dcf4ee8dc1 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2690,11 +2690,11 @@ static inline int mov_stsc_index_valid(unsigned int 
index, unsigned int count)
 /* Compute the samples value for the stsc entry at the given index. */
 static inline int64_t mov_get_stsc_samples(MOVStreamContext *sc, unsigned int 
index)
 {
-int chunk_count;
+unsigned int chunk_count = 0;
 
 if (mov_stsc_index_valid(index, sc->stsc_count))
 chunk_count = sc->stsc_data[index + 1].first - 
sc->stsc_data[index].first;
-else
+else if (sc->chunk_count >= sc->stsc_data[index].first)
 chunk_count = sc->chunk_count - (sc->stsc_data[index].first - 1);
 
 return sc->stsc_data[index].count * (int64_t)chunk_count;
-- 
2.20.1.611.gfbb209baf1-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mov: validate chunk_count vs stsc_data

[chcunningham]

Bad content may contain stsc boxes with a first_chunk index that
exceeds stco.entries (chunk_count). This ammends the existing check to
include cases where chunk_count == 0.
---
 libavformat/mov.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..2f3ad38ac3 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2694,8 +2694,11 @@ static inline int64_t 
mov_get_stsc_samples(MOVStreamContext *sc, unsigned int in
 
 if (mov_stsc_index_valid(index, sc->stsc_count))
 chunk_count = sc->stsc_data[index + 1].first - 
sc->stsc_data[index].first;
-else
+else {
+// Validation for stsc / stco  happens earlier in mov_read_stsc + 
mov_read_trak.
+av_assert0(sc->stsc_data[index].first <= sc->chunk_count);
 chunk_count = sc->chunk_count - (sc->stsc_data[index].first - 1);
+}
 
 return sc->stsc_data[index].count * (int64_t)chunk_count;
 }
@@ -4175,7 +4178,7 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
st->index);
 return 0;
 }
-if (sc->chunk_count && sc->stsc_count && sc->stsc_data[ sc->stsc_count - 1 
].first > sc->chunk_count) {
+if (sc->stsc_count && sc->stsc_data[ sc->stsc_count - 1 ].first > 
sc->chunk_count) {
 av_log(c->fc, AV_LOG_ERROR, "stream %d, contradictionary STSC and 
STCO\n",
st->index);
 return AVERROR_INVALIDDATA;
-- 
2.20.1.611.gfbb209baf1-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mov.c: require tfhd to begin parsing trun

[chcunningham]

Detecting missing tfhd avoids re-using tfhd track info from the previous
moof. For files with multiple tracks, this may make a mess of the
avindex and fragindex, which can later trigger av_assert0 in
mov_read_trun().
---
 libavformat/isom.h |  1 +
 libavformat/mov.c  | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index e629663949..69452cae8e 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -87,6 +87,7 @@ typedef struct MOVAtom {
 struct MOVParseTableEntry;
 
 typedef struct MOVFragment {
+int found_tfhd;
 unsigned track_id;
 uint64_t base_data_offset;
 uint64_t moof_offset;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..6afb656dae 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1366,6 +1366,9 @@ static void fix_frag_index_entries(MOVFragmentIndex 
*frag_index, int index,
 
 static int mov_read_moof(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
+// Set by mov_read_tfhd(). mov_read_trun() will reject files missing tfhd.
+c->fragment.found_tfhd = 0;
+
 if (!c->has_looked_for_mfra && c->use_mfra_for > 0) {
 c->has_looked_for_mfra = 1;
 if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
@@ -4544,6 +4547,8 @@ static int mov_read_tfhd(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 MOVTrackExt *trex = NULL;
 int flags, track_id, i;
 
+c->fragment.found_tfhd = 1;
+
 avio_r8(pb); /* version */
 flags = avio_rb24(pb);
 
@@ -4679,6 +4684,11 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 AVIndexEntry *new_entries;
 MOVFragmentStreamInfo * frag_stream_info;
 
+if (!frag->found_tfhd) {
+av_log(c->fc, AV_LOG_ERROR, "trun track id unknown, no tfhd was 
found\n");
+return AVERROR_INVALIDDATA;
+}
+
 for (i = 0; i < c->fc->nb_streams; i++) {
 if (c->fc->streams[i]->id == frag->track_id) {
 st = c->fc->streams[i];
-- 
2.20.1.611.gfbb209baf1-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

[chcunningham]

Codec information may change while reading ogg packets. Update the
stream's internal avctx to match.
---
 libavformat/oggparseogm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavformat/oggparseogm.c b/libavformat/oggparseogm.c
index a07453760b..b07a5d55ba 100644
--- a/libavformat/oggparseogm.c
+++ b/libavformat/oggparseogm.c
@@ -114,6 +114,9 @@ ogm_header(AVFormatContext *s, int idx)
 bytestream2_get_buffer(&p, st->codecpar->extradata, 
st->codecpar->extradata_size);
 }
 }
+
+// Update internal avctx with changes to codecpar above.
+st->internal->need_context_update = 1;
 } else if (bytestream2_peek_byte(&p) == 3) {
 bytestream2_skip(&p, 7);
 if (bytestream2_get_bytes_left(&p) > 1)
-- 
2.20.1.611.gfbb209baf1-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mov: validate chunk_count vs stsc_data

[chcunningham]

Bad content may contain stsc boxes with a first_chunk index that
exceeds stco.entries (chunk_count). This ammends the existing check to
include cases where chunk_count == 0. It also patches up the case
when stsc refers to unknown chunks, but stts has no samples (so we
can simply ignore stsc).
---
 libavformat/mov.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..45a40fe922 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2694,8 +2694,11 @@ static inline int64_t 
mov_get_stsc_samples(MOVStreamContext *sc, unsigned int in
 
 if (mov_stsc_index_valid(index, sc->stsc_count))
 chunk_count = sc->stsc_data[index + 1].first - 
sc->stsc_data[index].first;
-else
+else {
+// Validation for stsc / stco  happens earlier in mov_read_stsc + 
mov_read_trak.
+av_assert0(sc->stsc_data[index].first <= sc->chunk_count);
 chunk_count = sc->chunk_count - (sc->stsc_data[index].first - 1);
+}
 
 return sc->stsc_data[index].count * (int64_t)chunk_count;
 }
@@ -4167,6 +4170,13 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 
 c->trak_index = -1;
 
+// Here stsc refers to a chunk not described in stco. This is technically 
invalid,
+// but we can overlook it (clearing stsc) whenever stts_count == 0 
(indicating no samples).
+if (!sc->chunk_count && !sc->stts_count && sc->stsc_count) {
+sc->stsc_count = 0;
+av_freep(&sc->stsc_data);
+}
+
 /* sanity checks */
 if ((sc->chunk_count && (!sc->stts_count || !sc->stsc_count ||
 (!sc->sample_size && !sc->sample_count))) ||
@@ -4175,7 +4185,7 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
st->index);
 return 0;
 }
-if (sc->chunk_count && sc->stsc_count && sc->stsc_data[ sc->stsc_count - 1 
].first > sc->chunk_count) {
+if (sc->stsc_count && sc->stsc_data[ sc->stsc_count - 1 ].first > 
sc->chunk_count) {
 av_log(c->fc, AV_LOG_ERROR, "stream %d, contradictionary STSC and 
STCO\n",
st->index);
 return AVERROR_INVALIDDATA;
-- 
2.20.1.611.gfbb209baf1-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] libavformat/matroskadec: Add test for seeking with codec delay.

[chcunningham]

From: Chris Cunningham 

Also cleanup parens for the skip_to_timecode check.
---
 libavformat/matroskadec.c  |  2 +-
 tests/fate/seek.mak|  3 +++
 tests/ref/seek/mkv-codec-delay | 48 ++
 3 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 tests/ref/seek/mkv-codec-delay

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 60b1b34..d07a092 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -3153,7 +3153,7 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, uint8_t *data,
 // Compare signed timecodes. Timecode may be negative due to codec 
delay
 // offset. We don't support timestamps greater than int64_t anyway - 
see
 // AVPacket's pts.
-if ((int64_t)timecode < (int64_t)(matroska->skip_to_timecode))
+if ((int64_t)timecode < (int64_t)matroska->skip_to_timecode)
 return res;
 if (is_keyframe)
 matroska->skip_to_keyframe = 0;
diff --git a/tests/fate/seek.mak b/tests/fate/seek.mak
index b831cf8..f835da5 100644
--- a/tests/fate/seek.mak
+++ b/tests/fate/seek.mak
@@ -247,8 +247,11 @@ FATE_SEEK += $(FATE_SEEK_LAVF-yes:%=fate-seek-lavf-%)
 
 FATE_SEEK_EXTRA-$(CONFIG_MP3_DEMUXER)   += fate-seek-extra-mp3
 FATE_SEEK_EXTRA-$(call ALLYES, CACHE_PROTOCOL PIPE_PROTOCOL MP3_DEMUXER) += 
fate-seek-cache-pipe
+FATE_SEEK_EXTRA-$(CONFIG_MATROSKA_DEMUXER) += fate-seek-mkv-codec-delay
 fate-seek-extra-mp3:  CMD = run libavformat/tests/seek$(EXESUF) 
$(TARGET_SAMPLES)/gapless/gapless.mp3 -fastseek 1
 fate-seek-cache-pipe: CMD = cat $(TARGET_SAMPLES)/gapless/gapless.mp3 | run 
libavformat/tests/seek$(EXESUF) cache:pipe:0 -read_ahead_limit -1
+fate-seek-mkv-codec-delay:   CMD = run libavformat/tests/seek$(EXESUF) 
$(TARGET_SAMPLES)/mkv/codec_delay_opus.mkv
+
 FATE_SEEK_EXTRA += $(FATE_SEEK_EXTRA-yes)
 
 
diff --git a/tests/ref/seek/mkv-codec-delay b/tests/ref/seek/mkv-codec-delay
new file mode 100644
index 000..9d4582c
--- /dev/null
+++ b/tests/ref/seek/mkv-codec-delay
@@ -0,0 +1,48 @@
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret: 0 st:-1 flags:0  ts:-1.00
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret: 0 st:-1 flags:1  ts: 1.894167
+ret: 0 st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   
291
+ret: 0 st: 0 flags:0  ts: 0.788000
+ret: 0 st: 0 flags:1 dts: 0.794000 pts: 0.794000 pos:   7358 size:   
154
+ret: 0 st: 0 flags:1  ts:-0.317000
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret:-1 st:-1 flags:0  ts: 2.576668
+ret: 0 st:-1 flags:1  ts: 1.470835
+ret: 0 st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   
291
+ret: 0 st: 0 flags:0  ts: 0.365000
+ret: 0 st: 0 flags:1 dts: 0.374000 pts: 0.374000 pos:   3963 size:   
150
+ret: 0 st: 0 flags:1  ts:-0.741000
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret:-1 st:-1 flags:0  ts: 2.153336
+ret: 0 st:-1 flags:1  ts: 1.047503
+ret: 0 st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   
291
+ret: 0 st: 0 flags:0  ts:-0.058000
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret: 0 st: 0 flags:1  ts: 2.836000
+ret: 0 st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   
291
+ret:-1 st:-1 flags:0  ts: 1.730004
+ret: 0 st:-1 flags:1  ts: 0.624171
+ret: 0 st: 0 flags:1 dts: 0.614000 pts: 0.614000 pos:   5903 size:   
159
+ret: 0 st: 0 flags:0  ts:-0.482000
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret: 0 st: 0 flags:1  ts: 2.413000
+ret: 0 st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   
291
+ret:-1 st:-1 flags:0  ts: 1.306672
+ret: 0 st:-1 flags:1  ts: 0.200839
+ret: 0 st: 0 flags:1 dts: 0.194000 pts: 0.194000 pos:   2512 size:   
159
+ret: 0 st: 0 flags:0  ts:-0.905000
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret: 0 st: 0 flags:1  ts: 1.989000
+ret: 0 st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   
291
+ret: 0 st:-1 flags:0  ts: 0.883340
+ret: 0 st: 0 flags:1 dts: 0.894000 pts: 0.894000 pos:   8154 size:   
155
+ret: 0 st:-1 flags:1  ts:-0.222493
+ret: 0 st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:748 size:   
320
+ret:-1 st: 0 flags:0  ts: 2.672000
+ret: 0 st: 0 flags:1  ts: 1.566000
+ret: 0 st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   
291
+ret: 0 st:-1 flags:0  ts: 0.460008
+ret: 0 st: 0 flags:1 dts: 0.474000 pts: 0.474000 pos:   4768 size:   
153
+ret: 0 st:-1 flags:1  ts:-0.645825
+ret: 0  

[FFmpeg-devel] [PATCH] avformat/utils: Check negative bps before shifting in ff_get_pcm_codec_id()

[chcunningham]

From: Chris Cunningham 

Fixes: undefined shift.
---
 libavformat/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 5f5f03e..ad5cfa2 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -2955,7 +2955,7 @@ enum AVCodecID ff_codec_get_id(const AVCodecTag *tags, 
unsigned int tag)
 
 enum AVCodecID ff_get_pcm_codec_id(int bps, int flt, int be, int sflags)
 {
-if (bps > 64U)
+if (bps <= 0 || bps > 64U)
 return AV_CODEC_ID_NONE;
 
 if (flt) {
-- 
2.8.0.rc3.226.g39d4020

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/utils: Check negative bps before shifting in ff_get_pcm_codec_id()

[chcunningham]

From: Chris Cunningham 

Fixes: undefined shift.
---
 libavformat/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 5f5f03e..d1e4306 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -2955,7 +2955,7 @@ enum AVCodecID ff_codec_get_id(const AVCodecTag *tags, 
unsigned int tag)
 
 enum AVCodecID ff_get_pcm_codec_id(int bps, int flt, int be, int sflags)
 {
-if (bps > 64U)
+if (bps <= 0 || bps > 64)
 return AV_CODEC_ID_NONE;
 
 if (flt) {
-- 
2.8.0.rc3.226.g39d4020

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

[chcunningham]

From: Chris Cunningham 

"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

[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

[chcunningham]

From: Chris Cunningham 

"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 with TOC precision.
(see https://crbug.com/545914#c13)

For VBR, fast seek is not precise, so continue to prefer the TOC
when available (the lesser of two evils).

Also, some re-ordering of the logic in mp3_seek to simplify and
give usetoc=1 precedence over fastseek flag.
---
 libavformat/mp3dec.c | 45 +++--
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..a1f21b7 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -115,7 +115,8 @@ static void read_xing_toc(AVFormatContext *s, int64_t 
filesize, int64_t duration
 {
 int i;
 MP3DecContext *mp3 = s->priv_data;
-int fill_index = mp3->usetoc == 1 && duration > 0;
+int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
+int fill_index = (mp3->usetoc || fast_seek) && duration > 0;
 
 if (!filesize &&
 !(filesize = avio_size(s->pb))) {
@@ -344,9 +345,6 @@ static int mp3_read_header(AVFormatContext *s)
 int ret;
 int i;
 
-if (mp3->usetoc < 0)
-mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
-
 st = avformat_new_stream(s, NULL);
 if (!st)
 return AVERROR(ENOMEM);
@@ -501,40 +499,43 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 MP3DecContext *mp3 = s->priv_data;
 AVIndexEntry *ie, ie1;
 AVStream *st = s->streams[0];
-int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
-int64_t best_pos;
 int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
 int64_t filesize = mp3->header_filesize;
 
-if (mp3->usetoc == 2)
-return -1; // generic index code
-
 if (filesize <= 0) {
 int64_t size = avio_size(s->pb);
 if (size > 0 && size > s->internal->data_offset)
 filesize = size - s->internal->data_offset;
 }
 
-if (   (mp3->is_cbr || fast_seek)
-&& (mp3->usetoc == 0 || !mp3->xing_toc)
-&& st->duration > 0
-&& filesize > 0) {
-ie = &ie1;
-timestamp = av_clip64(timestamp, 0, st->duration);
-ie->timestamp = timestamp;
-ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
-} else if (mp3->xing_toc) {
+if (mp3->xing_toc && (mp3->usetoc || (fast_seek && !mp3->is_cbr))) {
+av_log(s, AV_LOG_ERROR, "XING seeking. filesize = %"PRId64"\n", 
filesize);
+// NOTE: The MP3 TOC is not a precise lookup table. The precision is
+// worse closer to the end of the file, especially for large files.
+// Seeking to 90% of duration in file of size 4M will be off by 
roughly 1 second.
+if (filesize > 400)
+av_log(s, AV_LOG_WARNING, "Using MP3 TOC to seek; may be 
imprecise.\n");
+
+int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
 if (ret < 0)
 return ret;
 
 ie = &st->index_entries[ret];
+} else if (fast_seek && st->duration > 0 && filesize > 0) {
+if (!mp3->is_cbr)
+av_log(s, AV_LOG_WARNING, "Using scaling to seek VBR MP3; may be 
imprecise.\n");
+
+ie = &ie1;
+timestamp = av_clip64(timestamp, 0, st->duration);
+ie->timestamp = timestamp;
+ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
 } else {
-return -1;
+return -1; // generic index code
 }
 
-best_pos = mp3_sync(s, ie->pos, flags);
+int64_t best_pos = mp3_sync(s, ie->pos, flags);
 if (best_pos < 0)
-return best_pos;
+  return best_pos;
 
 if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
 int frame_duration = av_rescale(st->duration, 1, mp3->frames);
@@ -546,7 +547,7 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 }
 
 static const AVOption options[] = {
-{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = -1}, -1, 2, AV_OPT_FLAG_DECODING_PARAM},
+{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM},
 { NULL },
 };
 
-- 
2.6.0.rc2.230.g3dd15c0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

[chcunningham]

From: Chris Cunningham 

"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 with TOC precision.
(see https://crbug.com/545914#c13)

For VBR, fast seek is not precise, so continue to prefer the TOC
when available (the lesser of two evils).

Also, some re-ordering of the logic in mp3_seek to simplify and
give usetoc=1 precedence over fastseek flag.
---
 libavformat/mp3dec.c| 44 +---
 libavformat/seek-test.c |  8 +---
 tests/fate/seek.mak |  2 +-
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..3c8c0ee 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -115,7 +115,8 @@ static void read_xing_toc(AVFormatContext *s, int64_t 
filesize, int64_t duration
 {
 int i;
 MP3DecContext *mp3 = s->priv_data;
-int fill_index = mp3->usetoc == 1 && duration > 0;
+int fast_seek = s->flags & AVFMT_FLAG_FAST_SEEK;
+int fill_index = (mp3->usetoc || fast_seek) && duration > 0;
 
 if (!filesize &&
 !(filesize = avio_size(s->pb))) {
@@ -344,9 +345,6 @@ static int mp3_read_header(AVFormatContext *s)
 int ret;
 int i;
 
-if (mp3->usetoc < 0)
-mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
-
 st = avformat_new_stream(s, NULL);
 if (!st)
 return AVERROR(ENOMEM);
@@ -501,40 +499,40 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 MP3DecContext *mp3 = s->priv_data;
 AVIndexEntry *ie, ie1;
 AVStream *st = s->streams[0];
-int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
-int64_t best_pos;
-int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
+int fast_seek = s->flags & AVFMT_FLAG_FAST_SEEK;
 int64_t filesize = mp3->header_filesize;
 
-if (mp3->usetoc == 2)
-return -1; // generic index code
-
 if (filesize <= 0) {
 int64_t size = avio_size(s->pb);
 if (size > 0 && size > s->internal->data_offset)
 filesize = size - s->internal->data_offset;
 }
 
-if (   (mp3->is_cbr || fast_seek)
-&& (mp3->usetoc == 0 || !mp3->xing_toc)
-&& st->duration > 0
-&& filesize > 0) {
-ie = &ie1;
-timestamp = av_clip64(timestamp, 0, st->duration);
-ie->timestamp = timestamp;
-ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
-} else if (mp3->xing_toc) {
+if (mp3->xing_toc && (mp3->usetoc || (fast_seek && !mp3->is_cbr))) {
+// NOTE: The MP3 TOC is not a precise lookup table. Accuracy is worse
+// for bigger files.
+av_log(s, AV_LOG_WARNING, "Using MP3 TOC to seek; may be 
imprecise.\n");
+
+int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
 if (ret < 0)
 return ret;
 
 ie = &st->index_entries[ret];
+} else if (fast_seek && st->duration > 0 && filesize > 0) {
+if (!mp3->is_cbr)
+av_log(s, AV_LOG_WARNING, "Using scaling to seek VBR MP3; may be 
imprecise.\n");
+
+ie = &ie1;
+timestamp = av_clip64(timestamp, 0, st->duration);
+ie->timestamp = timestamp;
+ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
 } else {
-return -1;
+return -1; // generic index code
 }
 
-best_pos = mp3_sync(s, ie->pos, flags);
+int64_t best_pos = mp3_sync(s, ie->pos, flags);
 if (best_pos < 0)
-return best_pos;
+  return best_pos;
 
 if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
 int frame_duration = av_rescale(st->duration, 1, mp3->frames);
@@ -546,7 +544,7 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 }
 
 static const AVOption options[] = {
-{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = -1}, -1, 2, AV_OPT_FLAG_DECODING_PARAM},
+{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM},
 { NULL },
 };
 
diff --git a/libavformat/seek-test.c b/libavformat/seek-test.c
index 1926f21..bfd06db 100644
--- a/libavformat/seek-test.c
+++ b/libavformat/seek-test.c
@@ -56,7 +56,7 @@ static void ts_str(char buffer[60], int64_t ts, AVRational 
base)
 int main(int argc, char **argv)
 {
 const char *filename;
-AVFormatContext *ic = NULL;
+AVFormatContext *ic = avformat_alloc_context();
 int i, ret, stream_id;
 int j;
 int64_t timestamp;
@@ -76,8 +76,10 @@ int main(int argc, char **argv)
 frame_count = atoi(argv[i+1]);
 } else if(!strcmp(argv[i], "-duration")){
 duration = atoi(argv[i+1]);
-} else if(!strcmp(argv[i], "-usetoc")) {
-av_dict_set(&format_opts, 

[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

[chcunningham]

From: Chris Cunningham 

"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 with TOC precision.
(see https://crbug.com/545914#c13)

For VBR, fast seek is not precise, so continue to prefer the TOC
when available (the lesser of two evils).

Also, some re-ordering of the logic in mp3_seek to simplify and
give usetoc=1 precedence over fastseek flag.
---
 libavformat/mp3dec.c| 42 --
 libavformat/seek-test.c |  8 +---
 tests/fate/seek.mak |  2 +-
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..3dc2b5c 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -115,7 +115,8 @@ static void read_xing_toc(AVFormatContext *s, int64_t 
filesize, int64_t duration
 {
 int i;
 MP3DecContext *mp3 = s->priv_data;
-int fill_index = mp3->usetoc == 1 && duration > 0;
+int fast_seek = s->flags & AVFMT_FLAG_FAST_SEEK;
+int fill_index = (mp3->usetoc || fast_seek) && duration > 0;
 
 if (!filesize &&
 !(filesize = avio_size(s->pb))) {
@@ -344,9 +345,6 @@ static int mp3_read_header(AVFormatContext *s)
 int ret;
 int i;
 
-if (mp3->usetoc < 0)
-mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
-
 st = avformat_new_stream(s, NULL);
 if (!st)
 return AVERROR(ENOMEM);
@@ -501,38 +499,38 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 MP3DecContext *mp3 = s->priv_data;
 AVIndexEntry *ie, ie1;
 AVStream *st = s->streams[0];
-int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
-int64_t best_pos;
-int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
+int fast_seek = s->flags & AVFMT_FLAG_FAST_SEEK;
 int64_t filesize = mp3->header_filesize;
 
-if (mp3->usetoc == 2)
-return -1; // generic index code
-
 if (filesize <= 0) {
 int64_t size = avio_size(s->pb);
 if (size > 0 && size > s->internal->data_offset)
 filesize = size - s->internal->data_offset;
 }
 
-if (   (mp3->is_cbr || fast_seek)
-&& (mp3->usetoc == 0 || !mp3->xing_toc)
-&& st->duration > 0
-&& filesize > 0) {
-ie = &ie1;
-timestamp = av_clip64(timestamp, 0, st->duration);
-ie->timestamp = timestamp;
-ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
-} else if (mp3->xing_toc) {
+if (mp3->xing_toc && (mp3->usetoc || (fast_seek && !mp3->is_cbr))) {
+// NOTE: The MP3 TOC is not a precise lookup table. Accuracy is worse
+// for bigger files.
+av_log(s, AV_LOG_WARNING, "Using MP3 TOC to seek; may be 
imprecise.\n");
+
+int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
 if (ret < 0)
 return ret;
 
 ie = &st->index_entries[ret];
+} else if (fast_seek && st->duration > 0 && filesize > 0) {
+if (!mp3->is_cbr)
+av_log(s, AV_LOG_WARNING, "Using scaling to seek VBR MP3; may be 
imprecise.\n");
+
+ie = &ie1;
+timestamp = av_clip64(timestamp, 0, st->duration);
+ie->timestamp = timestamp;
+ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
 } else {
-return -1;
+return -1; // generic index code
 }
 
-best_pos = mp3_sync(s, ie->pos, flags);
+int64_t best_pos = mp3_sync(s, ie->pos, flags);
 if (best_pos < 0)
 return best_pos;
 
@@ -546,7 +544,7 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 }
 
 static const AVOption options[] = {
-{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = -1}, -1, 2, AV_OPT_FLAG_DECODING_PARAM},
+{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM},
 { NULL },
 };
 
diff --git a/libavformat/seek-test.c b/libavformat/seek-test.c
index 1926f21..bfd06db 100644
--- a/libavformat/seek-test.c
+++ b/libavformat/seek-test.c
@@ -56,7 +56,7 @@ static void ts_str(char buffer[60], int64_t ts, AVRational 
base)
 int main(int argc, char **argv)
 {
 const char *filename;
-AVFormatContext *ic = NULL;
+AVFormatContext *ic = avformat_alloc_context();
 int i, ret, stream_id;
 int j;
 int64_t timestamp;
@@ -76,8 +76,10 @@ int main(int argc, char **argv)
 frame_count = atoi(argv[i+1]);
 } else if(!strcmp(argv[i], "-duration")){
 duration = atoi(argv[i+1]);
-} else if(!strcmp(argv[i], "-usetoc")) {
-av_dict_set(&format_opts, "usetoc", argv[i+1], 0);
+} else if(!strcmp(argv[i], "-fastseek")) {
+if (atoi(argv[i+1])) {
+ic->flags |= AVFMT_FL

[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

[chcunningham]

From: Chris Cunningham 

"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 with TOC precision.
(see https://crbug.com/545914#c13)

For VBR, fast seek is not precise, so continue to prefer the TOC
when available (the lesser of two evils).

Also, some re-ordering of the logic in mp3_seek to simplify and
give usetoc=1 precedence over fastseek flag.
---
 libavformat/mp3dec.c| 39 +++
 libavformat/seek-test.c |  8 +---
 tests/fate/seek.mak |  2 +-
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..04c9861 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -115,7 +115,8 @@ static void read_xing_toc(AVFormatContext *s, int64_t 
filesize, int64_t duration
 {
 int i;
 MP3DecContext *mp3 = s->priv_data;
-int fill_index = mp3->usetoc == 1 && duration > 0;
+int fast_seek = s->flags & AVFMT_FLAG_FAST_SEEK;
+int fill_index = (mp3->usetoc || fast_seek) && duration > 0;
 
 if (!filesize &&
 !(filesize = avio_size(s->pb))) {
@@ -344,9 +345,6 @@ static int mp3_read_header(AVFormatContext *s)
 int ret;
 int i;
 
-if (mp3->usetoc < 0)
-mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
-
 st = avformat_new_stream(s, NULL);
 if (!st)
 return AVERROR(ENOMEM);
@@ -501,35 +499,36 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 MP3DecContext *mp3 = s->priv_data;
 AVIndexEntry *ie, ie1;
 AVStream *st = s->streams[0];
-int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
 int64_t best_pos;
-int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
+int fast_seek = s->flags & AVFMT_FLAG_FAST_SEEK;
 int64_t filesize = mp3->header_filesize;
 
-if (mp3->usetoc == 2)
-return -1; // generic index code
-
 if (filesize <= 0) {
 int64_t size = avio_size(s->pb);
 if (size > 0 && size > s->internal->data_offset)
 filesize = size - s->internal->data_offset;
 }
 
-if (   (mp3->is_cbr || fast_seek)
-&& (mp3->usetoc == 0 || !mp3->xing_toc)
-&& st->duration > 0
-&& filesize > 0) {
-ie = &ie1;
-timestamp = av_clip64(timestamp, 0, st->duration);
-ie->timestamp = timestamp;
-ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
-} else if (mp3->xing_toc) {
+if (mp3->xing_toc && (mp3->usetoc || (fast_seek && !mp3->is_cbr))) {
+// NOTE: The MP3 TOC is not a precise lookup table. Accuracy is worse
+// for bigger files.
+av_log(s, AV_LOG_WARNING, "Using MP3 TOC to seek; may be 
imprecise.\n");
+
+int64_t ret = av_index_search_timestamp(st, timestamp, flags);
 if (ret < 0)
 return ret;
 
 ie = &st->index_entries[ret];
+} else if (fast_seek && st->duration > 0 && filesize > 0) {
+if (!mp3->is_cbr)
+av_log(s, AV_LOG_WARNING, "Using scaling to seek VBR MP3; may be 
imprecise.\n");
+
+ie = &ie1;
+timestamp = av_clip64(timestamp, 0, st->duration);
+ie->timestamp = timestamp;
+ie->pos   = av_rescale(timestamp, filesize, st->duration) + 
s->internal->data_offset;
 } else {
-return -1;
+return -1; // generic index code
 }
 
 best_pos = mp3_sync(s, ie->pos, flags);
@@ -546,7 +545,7 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 }
 
 static const AVOption options[] = {
-{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = -1}, -1, 2, AV_OPT_FLAG_DECODING_PARAM},
+{ "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), 
AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM},
 { NULL },
 };
 
diff --git a/libavformat/seek-test.c b/libavformat/seek-test.c
index 1926f21..bfd06db 100644
--- a/libavformat/seek-test.c
+++ b/libavformat/seek-test.c
@@ -56,7 +56,7 @@ static void ts_str(char buffer[60], int64_t ts, AVRational 
base)
 int main(int argc, char **argv)
 {
 const char *filename;
-AVFormatContext *ic = NULL;
+AVFormatContext *ic = avformat_alloc_context();
 int i, ret, stream_id;
 int j;
 int64_t timestamp;
@@ -76,8 +76,10 @@ int main(int argc, char **argv)
 frame_count = atoi(argv[i+1]);
 } else if(!strcmp(argv[i], "-duration")){
 duration = atoi(argv[i+1]);
-} else if(!strcmp(argv[i], "-usetoc")) {
-av_dict_set(&format_opts, "usetoc", argv[i+1], 0);
+} else if(!strcmp(argv[i], "-fastseek")) {
+if (atoi(argv[i+1])) {
+ic->flags |= AVFMT_FLAG_FAST_SEEK;
+}
 } else {
 argc = 1;
 }
diff --git a/tests/fate/see