New issue
Advanced search Search tips

Issue 649882 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

MSE MP4 32-bit version 0 MovieHeader duration field is not checked correctly for "unknown" duration

Project Member Reported by wolenetz@chromium.org, Sep 23 2016

Issue description

Additional side-effect: Low-delay mode for MSE MP4 not engaged if MovieHeader version = 0

I spotted suspicious code today; still need to fully confirm if this bug is indeed there.

If MovieHeader version is 0, then "duration" is 32-bits, and https://cs.chromium.org/chromium/src/media/formats/mp4/mp4_stream_parser.cc?q=mp4_stre&sq=package:chromium&l=420 doesn't detect that it is signalling "unknown duration" since it's looking for 64 set bits, not 32 bits in this case.

The MovieHeader parser 0-extends the 32-bit version 0 duration value into a 64-bit field, nor is the code at https://cs.chromium.org/chromium/src/media/formats/mp4/mp4_stream_parser.cc?q=mp4_stre&sq=package:chromium&l=420 aware of header version/# of valid bits in duration.
 
Note that "0" as duration *is* correctly detected as unknown. This bug is tracking the lack of 32-bits-set in version 0 header duration detected as "unknown" duration.
See also sample file (other than a decode error contained within it) in  bug 649864  where mvhd->version == 0 and duration == 32-bits all set.
Confirmed.
Fix is out for review: https://codereview.chromium.org/2368773002/
cc+=sandersd@ w.r.t.  bug 649864  coincidence. Offline, Dan also shared some useful media file inspection and adjustment references.

Cc: sande...@chromium.org
Labels: MSE-compat
MSE-compat due to "Unknown Duration" is web-app-visible (and adjusts MSE behavior like {set,clear}LiveSeekableRange logic.)
Status: Fixed (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74582e201d84a1c30258239798f6ad4fcde923e8

commit 74582e201d84a1c30258239798f6ad4fcde923e8
Author: wolenetz <wolenetz@chromium.org>
Date: Tue Sep 27 19:16:47 2016

MSE: Recognize "unknown duration" in ISOBMFF mvhd version 0, duration max uint32 case

Previously, an "unknown duration" signalled by a mvhd duration
field was only understood to be unknown if it were 0, or a
64-bit (version 1 mvhd) special value of all bits set. This change adds
the same special value handling for a 32-bit (version 0 mvhd).

This change corrects detection of unknown mp4 MSE durations in some
cases. Unknown duration is web-app-visible and changes MSE behavior.
Also, it's used as a signal to enable our experimental low-delay
video rendering path.

I used a hex editor create the new test media file from
bear-1280x720-av_frag.mp4: truncated the file to just the initialization
segment (~1.5k, compared to full original file ~750k), and modified
the mvhd duration field values to be all bits set instead of 0.

The new unit test fails (due the the max-uint32 value being interpreted
as a real duration) before the rest of the code changes were included
into this CL.

TEST=MP4StreamParserTest.UnknownDuration_V0_AllBitsSet
BUG= 649882 
R=xhwang@chromium.org

Review-Url: https://codereview.chromium.org/2368773002
Cr-Commit-Position: refs/heads/master@{#421291}

[modify] https://crrev.com/74582e201d84a1c30258239798f6ad4fcde923e8/media/formats/mp4/box_definitions.cc
[modify] https://crrev.com/74582e201d84a1c30258239798f6ad4fcde923e8/media/formats/mp4/box_definitions.h
[modify] https://crrev.com/74582e201d84a1c30258239798f6ad4fcde923e8/media/formats/mp4/mp4_stream_parser.cc
[modify] https://crrev.com/74582e201d84a1c30258239798f6ad4fcde923e8/media/formats/mp4/mp4_stream_parser_unittest.cc
[modify] https://crrev.com/74582e201d84a1c30258239798f6ad4fcde923e8/media/test/data/README
[add] https://crrev.com/74582e201d84a1c30258239798f6ad4fcde923e8/media/test/data/bear-1280x720-av_frag-initsegment-mvhd_version_0-mvhd_duration_bits_all_set.mp4

Sign in to add a comment