Closed GoogleCodeExporter closed 9 years ago
Original comment by damien.haynes@gmail.com
on 24 Nov 2008 at 1:37
Hi Zeflash, since you already have your hands dirty in the Fanart code do you
mind
having a look at adding this :)
It will save me a lot of time understanding all the fanart logic thats going on.
Original comment by damien.haynes@gmail.com
on 24 Nov 2008 at 9:56
Original comment by damien.haynes@gmail.com
on 27 Nov 2008 at 5:35
[deleted comment]
Actually there was not much to do to enable the fanart loading in the view. It
was
very sluggish so I called LoadFanart() on a separate thread in
OnSeriesSelected(). Is
there a better way?
I also want to put in an option to disable this from Configuration and Plugin
Options
menu.
Zeflash, I will need you to review the code as Im just to new to be playing
with threads.
Original comment by damien.haynes@gmail.com
on 27 Nov 2008 at 5:43
I don't think there is a need to use a thread in that case.
We just need a delayed loading scheme, so that we don't try to load a fanart
whenever
the selection changes.
Instead, we use a timer that will load the background like 500ms after the
selection
has stopped changing. This way, if you go through the list quickly you are not
slowed
down by the fanart loading.
This is usually done by starting a timer in the selectionchanged event. If the
timer
is already running, it's reset. Otherwise it starts.
Then on reception of the timer event, the fanart is loaded if available.
Original comment by zefl...@gmail.com
on 27 Nov 2008 at 7:09
Thanks Zeflash, I think fforde uses a timer in Moving Pictures but is now
planning to
move to another thread due to performance still not being as good as he hoped.
So
this might not be the best soln now.
I can have a play with putting it in a timer event (good experience for me in
any case).
I had already played with putting it in another thread, but due to my
inexperience as
as a developer I was a bit uncomfortable. mattsk88 did some testing and said it
worked well. I will commit the code in soon so you can review, because Im sure
things
can be done better and that I have made mistakes.
As I said, I will investigate Timers and do a comparison.
Probably a few other enhancements needed are, is scaling the Fanart to match the
resolution of the screen. No Point wasting resources loading a 1920x1080 image
on a
1280x720 screen or less. Also possibly add image quality settings for Fanart.
Original comment by damien.haynes@gmail.com
on 27 Nov 2008 at 11:33
See
http://code.google.com/p/moving-pictures/issues/detail?id=68&colspec=ID%20Type%2
0Status%20Priority%20Stars%20Milestone%20Owner%20Summary
for reference to moving pictures
Original comment by damien.haynes@gmail.com
on 27 Nov 2008 at 11:34
Original comment by damien.haynes@gmail.com
on 28 Nov 2008 at 5:47
Ok. Well I assume the bad performance comes from the loading time of the bitmap
then?
In that case, I guess a combination of the timer to delay the loading trigger
and
loading the fanart in the background could be a solution.
Problem is, I actually removed the background loading of the fanart because I
was
thinking that was the problem when it was disappearing. But it's easy enough to
put
back in. I haven't checked in my fixes yet ... <sigh>
Time is running out again for me.
So if you're working from the trunk sources, you are indeed using loadFanart in
TVPlugin, and it's already loading in the background. Correct?
In that case the changes are really to just add this timer and call loadfanart
in the
end - which will load the bitmap async.
About scaling. I don't see the point really, we are loading only one fanart at a
time. The difference between a fullHD and a HDReady, even uncompressed, is
pretty
minimal:
a FullHD is going to take 8.3MB of RAM, an HDReady 3,6MB. It's less than 5MB.
IMHO,
not something we'd want to spend time and code on.
Furthermore, about the series label. I was looking in that code while
addressing the
bug in mediaportal texture cache, and the default method is to load the series
banners, resize them and store them in the texture cache.
The alternative way (meaning, old, or how other plugins do it) is to let the
GUIListControl do the loading with the full sized banners.
You'd think it's a good idea to resize the banners, but we store ALL of them in
RAM.
The regular way stores only what's being shown - as GUIListControl deallocates
the
bitmaps of items not visible anymore when you scroll up and down.
In my case, with about 30 series listed, the cached way takes 20MB more RAM
than the
regular way. The more series you have, the more difference you'll get.
So I guess what I'm saying is that trying to optimize is good, but one has to
make
sure to weight the various costs.
Original comment by zefl...@gmail.com
on 28 Nov 2008 at 8:25
Original comment by damien.haynes@gmail.com
on 30 Nov 2008 at 11:04
Original comment by damien.haynes@gmail.com
on 30 Nov 2008 at 11:05
Im pretty sure loading of Fanart is done in the background, but this is only
when
loading the facade.
Displaying Fanart in the series page requires LoadFanart() in the
OnSeriesSelected
handler so I added that. This was not done in the background.
Would be good if you can check-in your changes, that way we can do some
testing. Are
you going on Holidays, you say time is running out?
Original comment by damien.haynes@gmail.com
on 30 Nov 2008 at 11:23
I'm pretty sure the whole loading process of the fanart is indeed done in the
background. Well was, as I changed that - but didn't check it in.
I really need to find some time to do it, but I have tight deadline at work,
and with
the holidays coming there are plenty of other stuff to do.
Have you checked in your changes? how can I test them? I'd like to merge
everything
together and make sure everything still works before I check in my changes.
I'll send an email around on the dev list for IMs, as it would be more
efficient to
chat directly over stuff like that.
Original comment by zefl...@gmail.com
on 2 Dec 2008 at 8:43
Yes my changes are checked in. There is a new option, accessible from the
Options
menu to enable fanart on the series page.
Here is a test dll of r407 if you want to test without a merge:
http://www.2shared.com/file/4382763/9560a69/MP-TVSeries.html
Original comment by damien.haynes@gmail.com
on 2 Dec 2008 at 9:19
Now since r408 I see:
2008-12-03 21:33:38.463431 [ERROR][20]: Exception
:System.NullReferenceException:
Object reference not set to an instance of an object.
at MediaPortal.GUI.Library.GUIImage.AllocResources()
2008-12-03 21:33:38.463431 [ERROR][20]: Exception :Object reference not set
to an
instance of an object.
2008-12-03 21:33:38.463431 [ERROR][20]: site :Void AllocResources()
2008-12-03 21:33:38.464431 [ERROR][20]: source :Core
2008-12-03 21:33:38.464431 [ERROR][20]: stacktrace: at
MediaPortal.GUI.Library.GUIImage.AllocResources()
Appears to occur when scrolling through facade with series fanart enabled.
Original comment by damien.haynes@gmail.com
on 3 Dec 2008 at 10:40
I have now integrated the new Fanart Fading that Moving Pictures uses and it
works
really well. Performance+Eye Candy!
There have been a few bugs I have noticed but nothing serious, I will commit
after I
have finished doing some more testing.
For more information see:
http://code.google.com/p/moving-pictures/source/browse/trunk/Cornerstone.MP/Asyn
cImageResource.cs?spec=svn323&r=323
http://code.google.com/p/moving-pictures/source/browse/trunk/Cornerstone.MP/Imag
eSwapper.cs?spec=svn323&r=323
Original comment by damien.haynes@gmail.com
on 4 Dec 2008 at 12:57
John from Moving Pictures:
"For good speed a core.dll patch will be required. See here:
http://forum.team-mediaportal.com/general-development-no-feature-request-here-
48/guiimage-bug-patch-49610/
The fix will still work though without the core.dll patch, it will just be a
bit
slower. Still though responsiveness of UI movement should be improved and with
backdrop fading support even with the unpatched core.dll. It will just be a tad
slower."
Original comment by damien.haynes@gmail.com
on 4 Dec 2008 at 1:02
Will close this now, and raise a new enhancement to use the Cornerstone.MP
classes
Original comment by damien.haynes@gmail.com
on 9 Dec 2008 at 1:05
Original issue reported on code.google.com by
matts...@gmail.com
on 21 Nov 2008 at 10:59