Closed Bug 1350250 Opened 3 years ago Closed 3 years ago

[Fennec][HLS] HLSResource should obtain buffer information from GeckoHlsPlayer.java

Categories

(Firefox for Android :: Audio/Video, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- disabled

People

(Reporter: kikuo, Assigned: JamesCheng)

References

Details

Attachments

(1 file)

A JNI Wrapper should be provided for HLSResource to obtain buffer information, i.e. buffered length, buffering status, isEnded, isExpectingMoreData...etc.

These information should be queried from our GeckoHlsPlayer singleton instance.
Blocks: 1350246
Depends on: 1350241
NOTE: The call sites for this java wrapper will be provided in Bug 1350246.
Attachment #8864736 - Flags: review?(jyavenard)
Attachment #8864736 - Flags: review?(jolin)
Comment on attachment 8864736 [details]
Bug 1350250 - Add a GeckoHlsResourceWrapper java glue.

https://reviewboard.mozilla.org/r/136392/#review139912

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsResourceWrapper.java:13
(Diff revision 2)
> +
> +import org.mozilla.gecko.annotation.WrapForJNI;
> +import org.mozilla.gecko.mozglue.JNIObject;
> +
> +public class GeckoHlsResourceWrapper {
> +    private static final String LOGTAG = "GeckoHlsResourceWrapper";

name of member variable should be prefixed.
Attachment #8864736 - Flags: review?(jyavenard) → review+
Comment on attachment 8864736 [details]
Bug 1350250 - Add a GeckoHlsResourceWrapper java glue.

https://reviewboard.mozilla.org/r/136392/#review141512

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsResourceWrapper.java:18
(Diff revision 2)
> +    private static final String LOGTAG = "GeckoHlsResourceWrapper";
> +    private static final boolean DEBUG = false;
> +    private GeckoHlsPlayer player = null;
> +    private boolean destroyed = false;
> +
> +    public static class HlsResourceCallbacks extends JNIObject

Please be sure native callbacks won't use native objects after free.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsResourceWrapper.java:50
(Diff revision 2)
> +    }
> +
> +    @WrapForJNI(calledFrom = "gecko")
> +    public static GeckoHlsResourceWrapper create(String url,
> +                                                 GeckoHlsPlayer.ResourceCallbacks callback) {
> +        GeckoHlsResourceWrapper wrapper = new GeckoHlsResourceWrapper(url, callback);

`return new ...`
Comment on attachment 8864736 [details]
Bug 1350250 - Add a GeckoHlsResourceWrapper java glue.

https://reviewboard.mozilla.org/r/136392/#review141938
Attachment #8864736 - Flags: review?(jolin) → review+
Comment on attachment 8864736 [details]
Bug 1350250 - Add a GeckoHlsResourceWrapper java glue.

https://reviewboard.mozilla.org/r/136392/#review139912

> name of member variable should be prefixed.

rename the variable with "m" prefix.
Comment on attachment 8864736 [details]
Bug 1350250 - Add a GeckoHlsResourceWrapper java glue.

https://reviewboard.mozilla.org/r/136392/#review141512

> Please be sure native callbacks won't use native objects after free.

thanks for reminding, we will make sure the callback never be invoked if the native object is being free.
Comment on attachment 8864736 [details]
Bug 1350250 - Add a GeckoHlsResourceWrapper java glue.

https://reviewboard.mozilla.org/r/136392/#review141512

> `return new ...`

fixed
Pushed by jacheng@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/55a1612e3e58
Add a GeckoHlsResourceWrapper java glue. r=jolin,jya
we may create a bug to solve Bug 1368925 by make @WrapForJNI java code independent.

just land this code and solve it in another bug.
Depends on: 1368925
No longer depends on: 1368925
Depends on: 1368925
https://hg.mozilla.org/mozilla-central/rev/55a1612e3e58
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
This was reverted for Beta 55 as part of bug 1368925. It remains enabled on Trunk, which is now tracking Gecko 56.
You need to log in before you can comment on or make changes to this bug.