| Message ID | 20210310160757.59501-1-derek.buitenhuis@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mov: Properly consider if the file is self-initializing when marking sidx reading complete | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Wed, 10 Mar 2021, Derek Buitenhuis wrote: > Files with the "dash" major brand are guaranteed to only have a single > initialization range for the whole file. We can check this and stop > appropriately - which is useful, as the existing check to see if the > offset reaches the end of the file is not sufficient. For example, > YouTube creates valid self-initializing ISOBMFF files that contain > a single 'uuid' box appended at the end, after the last 'moof', and > this check does not catch that, which causes, for example, probing > to traverse the entire file (every single 'moof'), which is extremely > slow over a network, which is the main place these self-initializing > fragmented ISOBMFF files would be used. This is the same behavior that > was addressed in 2ff3c466eca66bb8eb32bb41a4ce70fe285e3ea0 for live-style > fragmented files. > > Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> > --- > libavformat/mov.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 1c07cff6b5..67eae24e02 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -5052,6 +5052,7 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom) > AVStream *ref_st = NULL; > MOVStreamContext *sc, *ref_sc = NULL; > AVRational timescale; > + AVDictionaryEntry *major_brand = av_dict_get(c->fc->metadata, "major_brand", NULL, AV_DICT_MATCH_CASE); > > version = avio_r8(pb); > if (version > 1) { > @@ -5120,7 +5121,11 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom) > sc->has_sidx = 1; > > // See if the remaining bytes are just an mfra which we can ignore. > - is_complete = offset == stream_size; > + // > + // Also check to see if the file is Self-initializing, which guarantees > + // only a single initialization range is present for the whole file. > + // See: ISO-IEC 23009-1 Section 6.3.5. > + is_complete = offset == stream_size || (major_brand && !strncmp(major_brand->value, "dash", 4)); > if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL)) { > int64_t ret; > int64_t original_pos = avio_tell(pb); > -- > 2.30.0 LGTM, I guess the only question that remains is whether "dash" in the compatible brands can imply the same too. (Our own muxer never sets "dash" as the major brand, only as a compatible brand, and that only if we only produce one global sidx.) // Martin
On 12/03/2021 11:20, Martin Storsjö wrote: > LGTM, I guess the only question that remains is whether "dash" in the > compatible brands can imply the same too. (Our own muxer never sets "dash" > as the major brand, only as a compatible brand, and that only if we only > produce one global sidx.) I *think* should probably be checking both major_brand and compatible_brands, actually, yeah. Based on the wording in ISO/ETC 14496-12, I think this is the case. I will send a v2. - Derek
diff --git a/libavformat/mov.c b/libavformat/mov.c index 1c07cff6b5..67eae24e02 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -5052,6 +5052,7 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom) AVStream *ref_st = NULL; MOVStreamContext *sc, *ref_sc = NULL; AVRational timescale; + AVDictionaryEntry *major_brand = av_dict_get(c->fc->metadata, "major_brand", NULL, AV_DICT_MATCH_CASE); version = avio_r8(pb); if (version > 1) { @@ -5120,7 +5121,11 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->has_sidx = 1; // See if the remaining bytes are just an mfra which we can ignore. - is_complete = offset == stream_size; + // + // Also check to see if the file is Self-initializing, which guarantees + // only a single initialization range is present for the whole file. + // See: ISO-IEC 23009-1 Section 6.3.5. + is_complete = offset == stream_size || (major_brand && !strncmp(major_brand->value, "dash", 4)); if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL)) { int64_t ret; int64_t original_pos = avio_tell(pb);
Files with the "dash" major brand are guaranteed to only have a single initialization range for the whole file. We can check this and stop appropriately - which is useful, as the existing check to see if the offset reaches the end of the file is not sufficient. For example, YouTube creates valid self-initializing ISOBMFF files that contain a single 'uuid' box appended at the end, after the last 'moof', and this check does not catch that, which causes, for example, probing to traverse the entire file (every single 'moof'), which is extremely slow over a network, which is the main place these self-initializing fragmented ISOBMFF files would be used. This is the same behavior that was addressed in 2ff3c466eca66bb8eb32bb41a4ce70fe285e3ea0 for live-style fragmented files. Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> --- libavformat/mov.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)