-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
[Inputstream] Add IChapter interface #16581
Conversation
@peak3d I would like to make you aware of another need we have in Kodi to add (outside) support for chapters in VOD streams. We have a stream, we have some chapter metadata with names and offsets, and we would like to add this information to the ListItem (in Python) so the player can take them into account. The feature request in the Kodi forums is here: (Not completely sure how that relates to this PR though) |
I guess that this is already working somehow, but this is not my real area (ffmpeg demuxer) |
AFAIR it is only supported if the (local) file has existing markers in the video. At least there is no Python interface to add this to a ListItem that is documented. But I also did not open a feature request either, it was on my TODO list. |
@dagwieers I haven't found any python inerface for this, too, so a feature request would be the best way to step forward. |
@@ -61,6 +61,9 @@ extern "C" | |||
|
|||
/// supports interface ITime | |||
SUPPORTS_ITIME = (1 << 5), | |||
|
|||
/// supports interface ITime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment needs to be changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code style looks okay now.
Would be great if you could run git clang-format yourself for future PRs.
@@ -1919,7 +1919,8 @@ void CVideoPlayer::HandlePlaySpeed() | |||
m_VideoPlayerAudio->SendMessage(new CDVDMsgDouble(CDVDMsg::GENERAL_RESYNC, m_clock.GetClock()), 1); | |||
} | |||
else if (m_CurrentVideo.syncState == IDVDStreamPlayer::SYNC_WAITSYNC && | |||
m_CurrentVideo.avsync == CCurrentStream::AV_SYNC_CONT) | |||
(m_CurrentVideo.avsync == CCurrentStream::AV_SYNC_CONT || | |||
m_CurrentAudio.syncState == IDVDStreamPlayer::SYNC_INSYNC)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FernetMenta need your opinion here: I play stream with chapters from inputstream.adaptive, each chapter devided with an DMX_SPECIALID_STREAMCHANGE. Syncinc stalls between the chapters (depending the stream length up to ~10 seconds). In this case m_CurrentVideo.avsync is CCurrentStream::AV_SYNC_NONE during stall, means that it will never be set to CONT.
The code change is identical to what is done some lines above for Audio. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is definitely wrong. If your video stream stalls, you are most like doing something wrong setting the stream parameters. OpenDefaultStream / OpenVideoStream won't reopen video stream if config did not change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw. this is another example where you hide non trivial changes in some PR with a title nobody would expect those kind of changes in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FernetMenta I doubt that this error is caused by flags passed from outside.
Even if it would be like that, the combination m_CurrentVideo.syncState == IDVDStreamPlayer::SYNC_WAITSYNC and m_CurrentVideo.avsync == CCurrentStream::AV_SYNC_NONE should never happen, right?
Edit: If you have some spare time, It would be nice if you reproduce and break execution during the wait. kodi + this PR / inputstream.adaptive master branch (not Matrix) and VTM go addon (it provides chapter based streams). I have already checked PTS, they are fine.
Edit2: You'll have to revert this second commit to see the stall, and yes, it will be an own PR if things are clarified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it would be like that, the combination m_CurrentVideo.syncState == IDVDStreamPlayer::SYNC_WAITSYNC and m_CurrentVideo.avsync == CCurrentStream::AV_SYNC_NONE should never happen, right?
Wrong, this is quite a common state.
You should investigate why your streamchange puts VPV into state SYNC_WAITSYNC. In general that happens when a new video stream is opened. Why does this happen if parameters do not change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the state of avsync should not change that often as it does in your log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll first try to figure out why it changes that often
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://paste.ubuntu.com/p/TQVrbD7mHv/
The old log was not ok, sry.
The stall ist now here:
2019-09-21 21:51:40.834 T:4294967294 DEBUG: CVideoPlayer::HandleMessages - player started 2
Maybe the issue is caused by the very short length of the stream 2001/2, it is only a few seconds, but right ahead the stream which have the 7sec stall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can VP handle the situation that 3 streams are in "queue" ? we have 1001 playing, 2001 is very short, and we start already with 3001.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FernetMenta I created an example for reproduction, maybe it helps:
https://paste.ubuntu.com/p/JMqb4ptWCJ/, instructions in the paste.
Seems there is no version 2.5.1 available?! |
Had overlooked this request. The comments on #16977 (review) also then related to here. Maybe leave it then on Leia like on request, but on Matrix change to |
[Backport][Inputstream] Add IChapter interface #16581
This reverts commit da9b5f1. The API change on Leia has destroyed all after them created addon to use on older Leia versions.
Revert "[Backport][Inputstream] Add IChapter interface #16581"
Description
This PR adds the CDVDInputStream::IChapter interface to inputstream API interface
Motivation and Context
Mpeg DASH manifests can contain multiple periods. Each period is an independent stream, even DRM protection can differ between periods. Often periods contain advertisment.
Issue: xbmc/inputstream.adaptive#287
How Has This Been Tested?
Note
Types of change