1043710 - Expose h.264 frame reorder depth to PlatformDecoderModules

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Ralph Giles (:rillian)]

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.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Expose decode timestamp

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

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Expose decode timestamp

Comment 3

[Ralph Giles (:rillian)]

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?

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Yeah.. that ended up in the wrong patch :-)

Comment 5

[Ralph Giles (:rillian)]

Example test file for which our current 3-frame-queue doesn't work:

 https://people.mozilla.org/~ajones/mse/test.mp4

Comment 6

[Ralph Giles (:rillian)]

Attachment: Expose decode timestamp

Update patch to apply cleanly, remove spurious change.

Comment 7

[Ralph Giles (:rillian)]

Attachment: Pass dts to VideoToolbox

At last I can fill in the correct value here.

Comment 8

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8480141 [details] [diff] [review]
Expose decode timestamp

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

More kft's area.

Comment 9

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

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.

Comment 10

[Ralph Giles (:rillian)]

Attachment: Drain reorder buffer with timestamp comparison

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.

Comment 11

[Ralph Giles (:rillian)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/632d5a8709cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/feb4a53fd308
https://hg.mozilla.org/integration/mozilla-inbound/rev/674723cd981a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/632d5a8709cb
https://hg.mozilla.org/mozilla-central/rev/feb4a53fd308
https://hg.mozilla.org/mozilla-central/rev/674723cd981a