Closed Bug 1295901 Opened 4 years ago Closed 4 years ago

Change the semantics of MediaDecoderReader::ReleaseMediaResources()

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(1 file)

The function has 2 overrides:
One is MediaOmxReader which is used by B2G which we don't officially support. The other is MediaFormatReader which requires a Seek() instead of AsyncReadMetadata() to recover decoding.

I would like to change the comment to:
// Release decoder resources they should be released in dormant state
// The reader can resume decoding by calling Seek() to a specific position.

This change has some benefits:
1. Simplify the state transitions of MDSM which will only read metadata at most once.
2. Allow MDSM to recover from dormant state more quickly (without reading metadata again).
Assignee: nobody → jwwang
Blocks: 1286129, 1295892
Priority: -- → P3
Attachment #8781872 - Flags: review?(jyavenard)
Comment on attachment 8781872 [details]
Bug 1295901 - Change the semantics/naming of MediaDecoderReader::ReleaseMediaResources().

https://reviewboard.mozilla.org/r/72214/#review69778

that's it ?

if it's not about releasing media resource, the function name should be changed accordingly.
Attachment #8781872 - Flags: review?(jyavenard) → review-
Comment on attachment 8781872 [details]
Bug 1295901 - Change the semantics/naming of MediaDecoderReader::ReleaseMediaResources().

https://reviewboard.mozilla.org/r/72214/#review69778

How about "ReleaseDecoderResources"?
Comment on attachment 8781872 [details]
Bug 1295901 - Change the semantics/naming of MediaDecoderReader::ReleaseMediaResources().

https://reviewboard.mozilla.org/r/72214/#review69778

well, this is within a MediaDecoderReader, so the term reader appear more applicable. Otherwise it could be confused with the decoder part of MediaDecoder.

So ReleaseReaderResources

or simply, ReleaseResources.
Comment on attachment 8781872 [details]
Bug 1295901 - Change the semantics/naming of MediaDecoderReader::ReleaseMediaResources().

https://reviewboard.mozilla.org/r/72214/#review69778

The shorter is the better. I will take "ReleaseResources".
Comment on attachment 8781872 [details]
Bug 1295901 - Change the semantics/naming of MediaDecoderReader::ReleaseMediaResources().

https://reviewboard.mozilla.org/r/72214/#review69840
Attachment #8781872 - Flags: review?(jyavenard) → review+
Thanks!
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4030666f6c50
Change the semantics/naming of MediaDecoderReader::ReleaseMediaResources(). r=jya
https://hg.mozilla.org/mozilla-central/rev/4030666f6c50
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.