[FFmpeg-devel] [mov] See if mfra makes up the difference for an incomplete sidx.

[Derek Buitenhuis]

On 13/08/2020 23:03, Dale Curtis wrote:
> A few popular sites have started generating MP4 files which have a
> sidx plus an mfra. The sidx accounts for all size except the mfra,
> so the old code did not mark the fragment index as complete.
> 
> Instead we can just check if there's an mfra and if its size makes
> up the difference we can mark the index as complete.
> 
> Fixes: https://crbug.com/1107130
> Signed-off-by: Dale Curtis <dalecurtis at chromium.org>

Concept in itself seems OK, I think.

A few nits:

> -    if (offset == avio_size(pb)) {
> +    // See if the remaining bytes are just an mfra which we can ignore.
> +    is_complete = offset == stream_size;
> +    if (!is_complete) {

Patch lacks the context for stream_size here - will it always be
correct? Only asking since the check for the coede below this will
have changed from avio_size(pb) to stream_size.

> +        int64_t original_pos = avio_tell(pb);
> +        int32_t mfra_size;
> +        if (avio_seek(pb, stream_size - 4, SEEK_SET) < 0)
> +            return 0;

Can mov_read_sidx not return an error?

> +        mfra_size = avio_rb32(pb);
> +        if (offset + mfra_size == stream_size)
> +          is_complete = 1;

Needs two more spaces.

> +        avio_seek(pb, original_pos, SEEK_SET);
> +    }
> +
> +    if (is_complete) {

Can be an else statement.

Otherwise OK.

- Derek