[FFmpeg-devel,v2] avformat/mov: Properly consider if the file is self-initializing when marking sidx reading complete

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,
Premiere 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>
---
Updated the commit message too, to reflect that the files are actually
a YouTube-muxed file that was opened in Premiere, which appends XMP
metadata in a 'uuid' box to files it opens. Gross.
---
 libavformat/mov.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

On Fri, 12 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,
> Premiere 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>
> ---
> Updated the commit message too, to reflect that the files are actually
> a YouTube-muxed file that was opened in Premiere, which appends XMP
> metadata in a 'uuid' box to files it opens. Gross.
> ---
> libavformat/mov.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 23b0ead01e..0eb52ab6e6 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -5052,6 +5052,8 @@ 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);
> +    AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata, "compatible_brands", NULL, AV_DICT_MATCH_CASE);
>
>     version = avio_r8(pb);
>     if (version > 1) {
> @@ -5120,7 +5122,13 @@ 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)) ||
> +                  (compatible_brands && strstr(compatible_brands->value, "dash"));
>     if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL)) {
>         int64_t ret;
>         int64_t original_pos = avio_tell(pb);
> -- 
> 2.30.0

LGTM. The commit message still talks only about the major brand though.

// Martin

On 12/03/2021 18:49, Martin Storsjö wrote:
> LGTM. The commit message still talks only about the major brand though.

It seems at some point we added an unrelated test, seek-extra-mp4, which uses
an invalid dash-branded (under compatible brands) file generated by libavformat...
We mux correctly now, and for mos of git history, but it seems we maybe muxed
incorrectly at some point?

What's the best course of action here? Fix the sample with a few filename?
Drop compatible_brands (even though AFAIK, this is the correct way to handle
this)?

(Sample added in 37e8edc9f51545ad91cbdf7dbe796af93f011abe by Dale - I don't
know how he made it, but it was made with libavformat.)

- Derek