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

VideoPlayer: gapless playback on stream change #8886

Merged
merged 8 commits into from
Jan 17, 2016

Conversation

FernetMenta
Copy link
Contributor

this enables gapless video playback for decoders that are capable of drain and allow multiple instances.
credits to @mapfau who has been working with me on this.

@afedchin could you please check dxva if it needs adaption. Configure of renderer now works without an extra cycle
@popcornmix could you please check mmal

@FernetMenta FernetMenta added Type: Improvement non-breaking change which improves existing functionality v17 Krypton Component: Video labels Jan 16, 2016
@popcornmix
Copy link
Member

These commits have been in Milhouse builds for last couple of days.
We have one regression reported that may have been caused by this:
http://forum.kodi.tv/showthread.php?tid=250817&pid=2217441#pid2217441

These are the changes in the first build with the regression:
http://forum.kodi.tv/showthread.php?tid=250817&pid=2216328#pid2216328

@FernetMenta
Copy link
Contributor Author

All feedback to those changes older than one day are obsolete because of recent changes.

@FernetMenta
Copy link
Contributor Author

@popcornmix issues related to this PR may manifest themselves by rendering a black/green screen on stream change or not working renderer at all.

@popcornmix
Copy link
Member

I did re-pick the commits from this PR yesterday early evening. You can see the commits used in last night's build here: https://github.com/popcornmix/xbmc/commits/newclock5

Nothing obvious is broken on normal usage of MMAL. I think I have some samples with resolution changes in - I'll see if they are affected.

@FernetMenta
Copy link
Contributor Author

Windows tests were done by @mapfau

jenkins build this please

@FernetMenta
Copy link
Contributor Author

addon build errors on Windows unrelated.

FernetMenta added a commit that referenced this pull request Jan 17, 2016
VideoPlayer: gapless playback on stream change
@FernetMenta FernetMenta merged commit e9ecdb7 into xbmc:master Jan 17, 2016
@FernetMenta FernetMenta deleted the gapless branch January 17, 2016 20:04
@Voyager1
Copy link

Testing did apparently not include DVD menus... unfortunately since this PR they are broken. I used the SPONGEBOB https://www.dropbox.com/s/hquzbke1x6pvz4e/SPONGEBOB.zip?dl=0 and the TARZ https://www.dropbox.com/s/j22rlb9ur8e4n47/TARZ.zip?dl=0 examples. Problems include PINK stills (with DXVA rendering forced) and missing menu highlights. I am on a Jan 21 build and have reverted #8915, #8907, and #8886 (reverse chronological order of VideoPlayer changes).
UPDATE: also unresponsive DVD navigation is among the issues.

Before reverting, I have also tried the currently open #8935 but that didn't help...

@Voyager1
Copy link

update and confirmation: I refined my analysis and narrowed it down to ONLY this PR (reverted with very minimal conflict resolution).

Voyager1 referenced this pull request in afedchin/xbmc Jan 21, 2016
@FernetMenta
Copy link
Contributor Author

why don't you try to debug and narrow it down further?

@Voyager1
Copy link

2 things: time (may take a while) and while I can try you're obviously closer to the VideoPlayer code... Since I provided the examples that break, I thought that you may identify the issue really quickly. Anyway I'll take a look.

@FernetMenta
Copy link
Contributor Author

TARZ and spongebob do work fine here on OSX (#8935) included

EDIT: would you have the exact steps to reproduce?

@Voyager1
Copy link

I reverted only the WinRenderer commit and that is already slightly better. Now the menus are "working" but I still get the stills as full-screen PINK (in TARZ example).
Repro steps: for TARZ, just hit enter on the language screen, then at the next menu choose bonus material. Both the language screen and the bonus screen look entirely pink.

@FernetMenta
Copy link
Contributor Author

works without any issues here. since sw decoding is equal on our systems, the issue is most likely in Windows renderer. @afedchin could you have a look?

@Voyager1
Copy link

When I add #8935 the situation changes: sometimes the pink stills are gone (not always - seems random) and when that is the case, menu does not respond anymore - weird stuff

@ghost
Copy link

ghost commented Jan 21, 2016

The VideoRender thread seems to go away in some situations:

http://pastebin.com/srfbKdbX

There are stacktraces in my log folder each time there are artefacts or stills.

@afedchin
Copy link
Member

@mapfau why you post this here?

@ghost
Copy link

ghost commented Jan 21, 2016

This is the stacktrace playing the sponge example with the version including 8868.

  • play: VIDEO_TS.IFO
  • press Return on first menue

There is an access to an NULL buffer on WinRenderer.cpp:248
In my case source is 3 but the m_VideoBuffers[source] is NULL

Above is bullshit, but the picture_buffer seems to be not OK and produces exceptions when reading from it.

@afedchin
Copy link
Member

@mapfau your stack trace is related to converting DVDVideoPicture in WinRenderer for DXVA. If renderer receives a nullptr data then it crash. why it receives nullptr?

@ghost
Copy link

ghost commented Jan 21, 2016

Problem seems to be something with the decoder:

VideoPlayerVideo: 295 (I'll try to make a hyperlink...)
https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/VideoPlayerVideo.cpp#L295

The picture seem NOT to be allocated but is passed to the renderer

m_picture watch:
http://pastebin.com/d2gM8wz0

@afedchin
Copy link
Member

@mapfau I cannot reproduce an issue with sponge sample.

@ghost
Copy link

ghost commented Jan 21, 2016

it seems to be cpu dependend - sometimes I have pink fullscreen, sometimes green, and sometimes the exception. There is a change in the DXVA decoder for the last frames - I'll deactivate it and try again.
Are the problems also present using software decoding? Or is it a pure DXVA issue?

@afedchin
Copy link
Member

It already uses sw decoding, but you selected DXVA renderer in the settings.

@FernetMenta
Copy link
Contributor Author

I don't see issues here on OSX and Linux. Note that a frame, once submitted to renderer, can be renderer as often as renderer wants to do. Regardless of DVP_FLAG_ALLOCATED. pics are refcounted and as long as renderer has a ref they have to be valid.

@FernetMenta
Copy link
Contributor Author

@mapfau could you apply the change mentioned here? FernetMenta@29e3ed8#commitcomment-15597930

@ghost
Copy link

ghost commented Jan 21, 2016

Moving ManageTextures(); from Line 221 into Update makes it better ( i can start the video without exception now).
But I still have a red / pink flash when playing the license text section

@Voyager1
Copy link

I still managed to get the exception with the move.
First-chance exception at 0x023C9F5A in Kodi.exe: 0xC0000005: Access violation reading location 0x1A691000.
First-chance exception at 0x7441D8A8 in Kodi.exe: Microsoft C++ exception: access_violation at memory location 0x1600F06C.
It seems to be random - once it works ok with pink screen (but overlays visible), once still is ok but no overlays + exception.

@afedchin
Copy link
Member

@FernetMenta bug is float, sometimes winrenderer can not read picture->data[0]

@afedchin
Copy link
Member

First-chance exception at 0x016A1FA6 in Kodi.exe: 0xC0000005: Access violation reading location 0x0AFEB000.
0x0afeb000 Error reading

@FernetMenta
Copy link
Contributor Author

all of us using sw decoding?

@afedchin
Copy link
Member

I use sw decoding. no issue with hw decoder

@Voyager1
Copy link

SW decoding indeed. Renderer doesn't matter (same with DXVA / PS and Software renderer).

@FernetMenta
Copy link
Contributor Author

what happens if you disable deinterlacing?

@Voyager1
Copy link

hey! that worked... setting deinterlacing to off does the trick

@FernetMenta
Copy link
Contributor Author

thanks! now I have a pointer. videocodecffmpeg somehow must destruct the source DVP_FLAG_ALLOCATED indicates as save to copy from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Improvement non-breaking change which improves existing functionality v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants