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

[core/video]Disabled set_volume() in core.video.ffpyplayer play() function. Fix for #6210 #6246

Merged
merged 2 commits into from
Apr 15, 2019

Conversation

misl6
Copy link
Member

@misl6 misl6 commented Apr 10, 2019

As mentioned in issue #6210 , on Android the video and videoplayer works only once.

Actual implementation on the ffpyplayer provider in core.video.ffpyplayer sets the volume just after the creation of the MediaPlayer.

After some investigation, seems that, the second usage of the Video/VideoPlayer w/ ffpyplayer takes more time to instantiate the AudioTrack that it looks like to be async.

In order to get my app running correctly I've commented out (as proposed in this PR) the set_volume call in the play() function.

I think that is not to be considered as the final fix ( we need to investigate further on the ffpyplayer implementation ), but should be merged ASAP, in order to temporarily solve this issue for all the affected users.

Copy link

@Danapessach Danapessach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it locally after the build and it solved the issue

@matham
Copy link
Member

matham commented Apr 12, 2019

I'm not sure how disabling setting the volume solves the crash? I don't see any mention of that line in the error message. In any case, you can pass the volume parameter to ff_opts instead of calling set_volume, rather than just commenting it out.

See the option here: https://github.com/matham/ffpyplayer/blob/master/ffpyplayer/player/player.pyx#L245

@tito
Copy link
Member

tito commented Apr 12, 2019

Yep, don't merge this, because even if it solve a possible crash, it also break the wanted volume at the start.

tito
tito previously requested changes Apr 12, 2019
Copy link
Member

@tito tito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Find an alternative to ensure the volume of the video is the one set by the user, like ff_opts explained by @matham

@Danapessach
Copy link

Danapessach commented Apr 12, 2019

I'm quite sure the volume does't matter when the video doesn't work at all...
This is why it was suggested as a temporary solution.

@matham
Copy link
Member

matham commented Apr 12, 2019

It does work generally, just not when playing a second time, or a perhaps when playing a particular file. This will break it for everyone even for cases that do currently work because the volume will stop working.

@Danapessach
Copy link

The problem happened for me with all files - different videos, different formats...

For me the volume worked OK after implementing the fix with the original volume of the video.

For you, when you tried this fix the volume didn't work at all?

@misl6
Copy link
Member Author

misl6 commented Apr 14, 2019

@tito @matham I've added volume inff_opts.
Consider that as a temp fix, I've scheduled some big improvements to the provider.

@matham matham dismissed tito’s stale review April 15, 2019 21:46

Changes were implemented

@matham matham merged commit ef7f38b into kivy:master Apr 15, 2019
@matham matham changed the title Disabled set_volume() in core.video.ffpyplayer play() function. Fix for #6210 [core/video]Disabled set_volume() in core.video.ffpyplayer play() function. Fix for #6210 May 23, 2019
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.

4 participants