Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/ontimedmetadata android #707

Merged

Conversation

RWOverdijk
Copy link
Contributor

This PR adds support for getting the id3 tag (currently support for the single timecode) in android. I have tested it and it functions identical to the other platforms in the projects.

Notes

  • Don't merge yet. The min sdk version now is 23. I have to make this optional first, otherwise this is a BC break.
  • Any pointers on getting the tags in a "prettier" manner than this are welcome. I've spent a full working day on google trying to find a solution that's prettier, and all of them were worse than this one, which is more elegant in terms of performance and readability.

I'll be forking this module, with this PR merged as I can't wait for your feedback right now. I'll try to keep submitting PRs as long as I don't stray from the current codebase too far.

@RWOverdijk
Copy link
Contributor Author

This is working great for me. Any feedback?

@raihanabbas232
Copy link

raihanabbas232 commented Aug 8, 2017

@RWOverdijk thanks for your concern...looking for some way to test that 👍...
Secondly, do we have to make any changes in ReactVideoViewManager.Java class for MetaData???

@RWOverdijk
Copy link
Contributor Author

@raihanabbas232 All the changes required are in this PR.

@fozzle
Copy link

fozzle commented Sep 1, 2017

thanks for this work and hope this gets merged soon

@nromptea42
Copy link

Hi @RWOverdijk, did you manage to make the onTimedMetaData optional ? I'm looking to implement this in my app but I don't want to loose my users who uses api 22.
Thank you !

@RWOverdijk
Copy link
Contributor Author

Not sure I understand your question. This method worked for me and was optional.

@nromptea42
Copy link

nromptea42 commented May 11, 2018

I took the code as it is in this pull request and applied it on a fork. It works great on android >=6.0 but it crashes on android 5.1. Maybe I did something wrong but I don't know what.
You said in your note : The min sdk version now is 23. I have to make this optional first, otherwise this is a BC break.
I want to know if it is possible to use your modifications on android 5.1 without crashing (Of course, no timedmetadata will be triggered).

@Ziv-Barber
Copy link

Ziv-Barber commented May 17, 2018

Not working. I'm not getting any metadata in the ontimedmetadata callback.
But I may doing something wrong in the encoder.
I'm using my plugin in wowza to add a AMF metadata to the stream.
I'm not getting a call to onTimedMetaDataAvailable in the java code (using this PR)

@cobarx
Copy link
Contributor

cobarx commented Jun 10, 2018

Is there a stream I can use to test this? I've tried a couple HLS streams and not had any luck getting onTimedMetadata to fire on iOS either, so I don't think they'll work for testing this. I'm totally not up on using timed metadata so a complete set of instructions would be great.

I can handle making this backwards compatible once I understand how to test this.

@cobarx
Copy link
Contributor

cobarx commented Jun 25, 2018

@RWOverdijk Ok I've updated the code so that you don't need to update to SDK 23. We don't want to make 23 the minimum as many people still want to support 4.1 or 4.4.

I've tried to test this with the only timed metadata stream I have and the listener is not detecting it. The URL is:
https://raw.githubusercontent.com/jwplayer/jwdeveloper-demos/master/demos/basic/audio-metadata/assets/index.m3u8
The same URL works fine on ExoPlayer and iOS.

Between only supporting newer devices, not detecting all metadata, and having to do major hacks to format the output, I'm not sure this is very workable. Is it worth supporting this or should we tell people to use ExoPlayer if they need timed metadata?

@cobarx cobarx merged commit 4fc0aab into TheWidlarzGroup:master Aug 7, 2018
@RWOverdijk RWOverdijk deleted the feat/ontimedmetadata-android branch August 24, 2018 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants