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

Sideload audio #4390

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Sideload audio #4390

wants to merge 2 commits into from

Conversation

pigeonmal
Copy link

Summary

A source.audioTracks for sideload audio tracks.
I updated the patch for work on 0.77 react-native.

Motivation

#4374
#3265

Changes

Get sideload audio and merge all in one source for exoplayer, check commits.

Test plan

It's not perfect for now, for example in my code I didn't setup language of audio in the Reactexoplayerview.java.
In 6.8.2 I had a poster bug, when audio tracks and poster are set : Audio work, but the poster stay over the video.
I don't test in 6.9 if this is fixed, I will test.

@freeboub
Copy link
Collaborator

Can you please add a section in documentation please ? I would like to test your change in source creation (side loaded subtitles) but it looks great 👍 thank you !

@pigeonmal
Copy link
Author

No problem, but I have 3 problems :

  1. The name of the prop is source.audioTracks not just audioTrack. Except that for now in the documentation, there is no grouping of source props. So this can lead to confusion?

  2. In sampleMimeType, how to add all available mime types check: https://github.com/androidx/media/blob/release/libraries/common/src/main/java/androidx/media3/common/MimeTypes.java

  3. In my code I put that title, language, mimetype are not mandatory, because I have implemented only the mimetype for now. Should they be mandatory?

Sorry for my bad English, I am French

@freeboub
Copy link
Collaborator

1/ there is a source prop here : https://github.com/TheWidlarzGroup/react-native-video/blob/master/docs/pages/component/props.mdx#source
But I agree this part of doc shall be clean up! it is already confusing :)
Bad doc is bette than no doc !

2/ The easiest is to redirect to exoplayer doc, no issue with that !

3/ you mean in that place:

            MediaItem audioItem = new MediaItem.Builder()
                            .setUri(track.getUrl())
                            .setMimeType(track.getSampleMimeType())
                            .build();

you can add setTitle and/or setLanguage of something like this ?
If that is the case, just add it and it should work !

4/ pas de soucis avec ton anglais, je cause franglais aussi :D

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.

2 participants