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

Already on GitHub? Sign in to your account

Slideshow picture with video rework #2580

Merged
merged 26 commits into from May 2, 2013

Conversation

Projects
None yet
Contributor

ulion commented Apr 9, 2013

This PR fixed some issues in slideshow/picture window:

  1. unify slideshow/picture window behavior.
  2. video in slideshow/picture nav now works as iDevice's photo app. when nav, show video thumb with a big play button.
  3. better error handling, and skip error/unplayable picture during slideshow.

for touch screen, need this too: xbmc/skin.touched#5

this one also included #2369 and #2573
and could consider as a rework and super version of #2164

besides, this one need #2244 to do better with video.

Owner

Memphiz commented Apr 9, 2013

Nice (didn't compile and try yet - lack of time). But i guess there will be users which will complain like "the slideshow stops on movies and i have to do something to make them play everytime a movie comes up" ;)

Contributor

ulion commented Apr 9, 2013

updated.

Owner

Montellese commented Apr 10, 2013

Any reason why this PR contains the OnPlay announcement change that you already opened #2573 for?
I haven't taken a look at the code but the slideshow code is a mess in general so I'm sure you've added some much needed and nice improvements.

Contributor

ulion commented Apr 10, 2013

I just learned from the mess what it intent to do and do it in more proper code.
#2573 is standalone with the whole change set, but as a super set of slideshow fix branch, it is included here.

Owner

Memphiz commented Apr 10, 2013

doesn't compile for me:

PlayBackRet missing and the PLAYBACK_OK stuff isn't found either.

Owner

Memphiz commented Apr 10, 2013

nvm - forgot to pull #2244

Owner

Memphiz commented Apr 10, 2013

Thats awesome work ulion as usual. I see that the play button for the videos doesn't appear when slideshow is active - so you already took care of my theoretical issue.

Works as described on ios. And it looks like it also fixes the direction issue when changing direction (former due to caching it might have loaded the wrong "next" image - there is even a trac ticket about that somewhere).

Contributor

ulion commented Apr 11, 2013

the reload on zoom code seems not be used for some time, I'd like to remove them.

Member

jmarshallnz commented Apr 11, 2013

The changes look fine at a brief scan. Can't really make it any worse than it is atm :)

ulion added some commits Mar 4, 2013

@ulion ulion Fix video in slideshow not show and hang in black problem. 67ec367
@ulion ulion [Picture] add methods to CSlideShowPic for reuse a loaded picture. c6446e2
@ulion ulion [SlideShow] use GetNextSlide() to get next slide number as long as po…
…ssible.
a361532
@ulion ulion Slideshow show video thumb when pausing. e6497fa
@ulion ulion Allow screen saver when slideshow pausing. a167241
@ulion ulion Consider 'unplayable' property when GetNextSlide(). 9d6fad1
@ulion ulion Try to skip to next when playing slideshow and current slide is 'unpl…
…ayable'.
afa7c02
@ulion ulion Slideshow check picture path on picture loaded. 95f62bd
@ulion ulion Add method to get slideshow number and pic slot for the background pi…
…c loader.
e03254e
@ulion ulion Only update texture when was loaded for reload. 37b9d0c
@ulion ulion No need to close next loaded image when zoom/reload current one. 1199a0c
@ulion ulion Only reload current image when not switching to next and current is n…
…ot video.
6eb4757
@ulion ulion Move display effect code into standalone method GetDisplayEffect. 477ad58
@ulion ulion Always close next loaded pic if not matching slidenumber. 745edfc
@ulion ulion Rewrite code for switching to next slide. e383b1e
@ulion ulion Slideshow rewrite the error handling code. 746e7b4
@ulion ulion Change CGUIWindowSlideShow::Select() to use same condition with code
loading first picture in Process().
c8167f9
@ulion ulion Change AnnouncePlayerPlay to announce speed == 0 for pausing slidesho…
…w or picture.
54fc0f4
@ulion ulion Keep slide infos during video playback. 0761185
@ulion ulion Unify picture display effect during slideshow or pausing, browsing. 156edce
@ulion ulion Add big play button for video in slideshow/picture window. 010b3d5
@ulion ulion Set direction to forward when starting a slideshow by action. cc769b0
@ulion ulion Allow change slideshow direction. b2c601c
@ulion ulion Slideshow reduce race condition when background loading picture. 3de411d
@ulion ulion Remove whole reload picture on zoom code, since we do not really use it. 4b12bb6
@ulion ulion rename IsDisplayEffectNeedChange to DisplayEffectNeedChange. 6940ff8
Contributor

ulion commented May 2, 2013

rebased.

Owner

Memphiz commented May 2, 2013

@jmarshallnz we hit that button do we? :)

@jmarshallnz jmarshallnz added a commit that referenced this pull request May 2, 2013

@jmarshallnz jmarshallnz Merge pull request #2580 from ulion/slideshow_picture_video_rework
Slideshow picture with video rework
40dabc2

@jmarshallnz jmarshallnz merged commit 40dabc2 into xbmc:master May 2, 2013

@Voyager1 Voyager1 commented on the diff May 2, 2013

xbmc/pictures/GUIWindowSlideShow.cpp
@@ -1001,50 +1083,66 @@ void CGUIWindowSlideShow::Move(float fX, float fY)
}
}
-void CGUIWindowSlideShow::OnLoadPic(int iPic, int iSlideNumber, CBaseTexture* pTexture, bool bFullSize)
+bool CGUIWindowSlideShow::PlayVideo()
+{
+ CFileItemPtr item = m_slides->Get(m_iCurrentSlide);
+ if (!item || !item->IsVideo())
+ return false;
+ CLog::Log(LOGDEBUG, "Playing current video slide %s", item->GetPath().c_str());
+ m_bPlayingVideo = true;
+ PlayBackRet ret = g_application.PlayFile(*item);
@Voyager1

Voyager1 May 2, 2013

Member

doesn't compile on Windows. looks like this type doesn't exist.

@ulion 'PlayBackRet'?
'PLAYBACK_OK', 'PLAYBACK_FAIL', 'PLAYBACK_CANCELED'?
This breaks compilation, at least on win32.

Contributor

ulion commented May 2, 2013

ah, we forgot to highlight, this one require #2244 be merged first... what shall we do now? @elupus

Owner

Memphiz commented May 3, 2013

2 Options. Either pull out the needed defines from #2244 to get master compiling again. Or if elupus finds time to review it today - we could wait. But imo the non-compiling state has to be held as short as possible.

Contributor

ulion commented May 3, 2013

I just pushed a compile fix commit to workaround it by using current PlayFile bool return value: bb75ae7

Thanks all for the efforts to improve Pictures. To echo the comment from Memphiz, while the big play button is gone during playback the experience still shows the large video thumb before playing the video. This results in very stilted playback. When a slideshow switches from a picture (or another video) to a new video, it flashes the thumbnail for a second before rolling the movie. The thumb never seems to be the first frame...so the experience is rather odd. I haven't looked at the code yet, but does anyone have any initial reactions on whether this will be fixable or are we going to have a new user experience in slideshows from Gotham forward?

Separately, I've been testing allowing video file types to be added to Player.Open and Playlist.Add json-rpc calls through modifications in PlaylistOperations.cpp and PlayerOperations.cpp so apps can start slideshows that include videos the same way the GUI does. Let me know if anyone has any interest in any of this work...

Contributor

ulion commented Jul 5, 2013

ok, I will fix that, thanks for point it out.

Thanks ulion. That makes a huge difference.

DeniR commented Jul 21, 2013

Slideshow in videos not work at all in 12.2 with MTS from sony hx9v. XBMC show big black/white picture like qr-code.

Contributor

t-nelson commented Jul 21, 2013

This isn't a critical fix and hence was not and will not be backported to
the 12.x series code base. You'll have to wait for 13.0.

DeniR commented Jul 22, 2013

Why in every version of XBMC there is a big problem with video slideshow ? Every version, at every years can't simple play video without interrupts or without skips. Old XMBC simple skips some videos from folder, this 12 version can't play at all, 13 nightly simple crushed while navigating to picture folder.
But all perfectly player, if i put all videos in m3u text file.
None of developers never try to play all vacations videos from point&shout cameras as is ?

Owner

MartijnKaijser commented Jul 22, 2013

@DeniR
This is not the support section. If you have something to ask use the forum.

Contributor

ulion commented Jul 22, 2013

if you found the 13 nightly crash, please init a Bug
Reporthttp://wiki.xbmc.org/?title=HOW-TO_Submit_a_Proper_Bug_Report
on: http://trac.xbmc.org/

Contributor

ulion commented Jul 23, 2013

@MartijnKaijser the crash in the ticket is introduced by #2890 obviously, so for stable, use the monthly build of last month and wait the current problem be fixed.

Member

arnova commented Jul 23, 2013

@MartijnKaijser / @ulion: Think I found the issue. PR coming up.

Member

arnova commented Jul 23, 2013

@MartijnKaijser : Please test #2998

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment