Closed Bug 1043710 Opened 6 years ago Closed 6 years ago

Expose h.264 frame reorder depth to PlatformDecoderModules

Categories

(Core :: Audio/Video, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: rillian, Assigned: ajones)

References

()

Details

Attachments

(3 files, 2 obsolete files)

The Apple VideoToolbox decoder needs to reorder decoded frames itself before returning them. We need to read and return the maximum reorder-depth from the file header to avoid buffering more decoded data than necessary.

This bug is about returning the value through the libstagefright binding's mp4_demuxer::VideoDecoderConfig, and adding code to read it from mp4 files if necessary.
Attached patch Expose decode timestamp (obsolete) — Splinter Review
You should be able to keep decoding frames until the decode timestamp is greater than or equal to the earliest presentation timestamp in the reorder queue
Assignee: nobody → ajones
Status: NEW → ASSIGNED
Attachment #8478055 - Attachment is obsolete: true
Comment on attachment 8478090 [details] [diff] [review]
Expose decode timestamp

Review of attachment 8478090 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libstagefright/binding/MoofParser.cpp
@@ +122,5 @@
>        tfhd = Tfhd(box, aTrex);
>      } else if (box.IsType("tfdt")) {
> +      if (!aTrex.mTrackId || tfhd.mTrackId == aTrex.mTrackId) {
> +        tfdt = Tfdt(box);
> +      }

This looks like an unrelated change?
Yeah.. that ended up in the wrong patch :-)
Example test file for which our current 3-frame-queue doesn't work:

 https://people.mozilla.org/~ajones/mse/test.mp4
Update patch to apply cleanly, remove spurious change.
Attachment #8478090 - Attachment is obsolete: true
Attachment #8480141 - Flags: review?(edwin)
At last I can fill in the correct value here.
Attachment #8480143 - Flags: review?(ajones)
Comment on attachment 8480141 [details] [diff] [review]
Expose decode timestamp

Review of attachment 8480141 [details] [diff] [review]:
-----------------------------------------------------------------

More kft's area.
Attachment #8480141 - Flags: review?(edwin) → review?(ajones)
Comment on attachment 8480141 [details] [diff] [review]
Expose decode timestamp

Review of attachment 8480141 [details] [diff] [review]:
-----------------------------------------------------------------

Oops, didn't realise this *was* kft's patch uploaded by Ralph.
Attachment #8480141 - Flags: review?(ajones) → review+
Implement Anthony's idea:

Instead of reading the reorder depth from the file, we can just rely on decode timestamp ordering of the samples. When a frame in the reorder buffer has a pts <= the dts of the frame we just inserted, we know we've seen all the frames we need to reorder.
Attachment #8480199 - Flags: review?(ajones)
Attachment #8480143 - Flags: review?(ajones) → review+
Attachment #8480199 - Flags: review?(ajones) → review+
You need to log in before you can comment on or make changes to this bug.