Closed Bug 1380568 Opened 2 years ago Closed 2 years ago

Remove AbstractMediaDecoder::NotifyDecodedFrames()

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(2 files)

No description provided.
Assignee: nobody → jwwang
Blocks: 1378295
Priority: -- → P3
In order to move bug 1378295 forward, we still have some minimum changes to AndroidMediaReader/Decoder while bug 1379190 is pending.

Hi Jya,
Can you review the changes of this bug or bug 1379190 will be landed soon?
Flags: needinfo?(jyavenard)
bug 1379190 will go in very shortly...
Flags: needinfo?(jyavenard)
Attachment #8887366 - Flags: review?(jyavenard)
Attachment #8887367 - Flags: review?(jyavenard)
Comment on attachment 8887366 [details]
Bug 1380568. P1 - store FrameStatistics in MFR.

https://reviewboard.mozilla.org/r/158210/#review165032

This is starting to become very cumbersome. It will be very painful to add any new decoders with all those arguments to think about...

::: dom/media/MediaFormatReader.cpp:2154
(Diff revision 2)
>      || (!decoder.mOutput.Length() && !decoder.mQueuedSamples.Length()),
>      "No frames can be demuxed or decoded while an internal seek is pending");
>  
>    // Record number of frames decoded and parsed. Automatically update the
>    // stats counters using the AutoNotifyDecoded stack-based class.
> -  AbstractMediaDecoder::AutoNotifyDecoded a(mDecoder);
> +  AbstractMediaDecoder::AutoNotifyDecoded a(mFrameStats);

how does this compile? does this change depends on another?

AutoNotifyDecoded constructor takes a AbstractMediaDecoder* as argument
Comment on attachment 8887367 [details]
Bug 1380568. P2 - remove AbstractMediaDecoder::NotifyDecodedFrames().

https://reviewboard.mozilla.org/r/158212/#review165034
Attachment #8887367 - Flags: review?(jyavenard) → review+
Comment on attachment 8887366 [details]
Bug 1380568. P1 - store FrameStatistics in MFR.

https://reviewboard.mozilla.org/r/158210/#review165192
Attachment #8887366 - Flags: review?(jyavenard) → review+
Thanks!
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25aa6acbfce4
P1 - store FrameStatistics in MFR. r=jya
https://hg.mozilla.org/integration/autoland/rev/931a09ec5d0f
P2 - remove AbstractMediaDecoder::NotifyDecodedFrames(). r=jya
https://hg.mozilla.org/mozilla-central/rev/25aa6acbfce4
https://hg.mozilla.org/mozilla-central/rev/931a09ec5d0f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.