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

[arm] enable pause/unpause of thumbjobs when playback starts/stops/ends. #1035

Merged
merged 0 commits into from
Jun 5, 2012

Conversation

davilla
Copy link
Contributor

@davilla davilla commented Jun 1, 2012

arm platform just does not have the ponies to run thumbjobs and handle playback. It would be nicer to get into dvdplayer and wait for thumbjobs to pause before queuing up playback but I know that's not going to happen :)

@jmarshallnz
Copy link
Contributor

Are the changes to ThumbLoader.cpp required? Doesn't pausing jobs simply prevent the jobs from running, so you don't need to stop adding new ones I think? I can't recall if the thumbloader thread stops on exit from the window.

Also, is this needed for all arm platforms, including quad core arm processors and the like? Perhaps it may be better to apply it across the board - they'll just extract next time around if they're not done this time around.

@davilla
Copy link
Contributor Author

davilla commented Jun 2, 2012

yes, when you enter a dir, ALL the items are queued to run. So they are past the pause point. I know, it's strange but that's the way it seems to work.

the only quad core arm processors out is tegra3 and it does not have neon so thumb extract is still going to be slow and a thief of cpu.

I figure we hit all arm right now, then think about tuning it for specifics. For example, single core atom/pentium-m might be a good inclusion.

@jmarshallnz
Copy link
Contributor

Once the jobmanager is paused, it should result in a single job running atm (possibly) that will finish, then an additional job added to the jobmanager's queue but not run. There will be a heap queued up in the CThumbLoader's CJobQueue though - this shouldn't matter as they're waiting on the one in the jobmanager queue.

Thus, the change in DoWork() might catch the one that is running but not finished (if you're really lucky). The changes in LoadItem() might potentially catch a job or two not being added to the thumbloader's CJobQueue (but it's likely that a bunch of earlier ones will have been added anyway). It'll catch only a few, as the thumbloader thread is stopped when we exit the videos menu to go to fullscreen.

As for the ifdef - IMO just cancel them regardless - I don't see a point in keeping the extraction running.

@davilla
Copy link
Contributor Author

davilla commented Jun 2, 2012

Cory and I ran into the case where thumb extractions that were queued kept running unless the were bailed in the DoWork. It's very strange and something that you don't see unless you actually look for it on desktop, but arm embedded shows it right up.

I'd need a sign off from spiff on removing the ifdef, his concern was backing out to home and into video content while playing a video would prevent further thumb extraction.

@jmarshallnz
Copy link
Contributor

That's odd, as the code is pretty clear: If the job has PRIORITY_LOW and matches a paused job type then PopJob never moves it off the rack. Both conditions seem satisfied here?

If jobs are being run, it suggests that there's a bug in the jobmanager's paused stuff. Can you add some breakpoints on the return false's and get any hits?

@huceke
Copy link
Contributor

huceke commented Jun 4, 2012

Another nice feature for arm would be the posability to limit the number of loaded images. Have seen it on the PI when we use a view with lot of fanarts we run pretty fast out of GPU memory.

@davilla davilla merged commit 0f2f1de into xbmc:master Jun 5, 2012
@davilla
Copy link
Contributor Author

davilla commented Jun 5, 2012

crap, wtf is git trying to do to me :)

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.

3 participants