Remove unused Inputstream addon functions and increase max props #17620

phunkyfish · 2020-04-05T19:55:05Z

Description

Remove the unused function CanPauseStream and CanSeekStream from the Inputstream addon API.
Increases the max number of stream properties

Motivation and Context

Not enough properties for inputstream.ffmpegdirect.

How Has This Been Tested?

On inputstream.ffmpegdirect on OSX

Screenshots (if appropriate):

Types of change

Bug fix (non-breaking change which fixes an issue)
Clean up (non-breaking change which removes non-working, unmaintained functionality)
Improvement (non-breaking change which improves existing functionality)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that will cause existing functionality to change)
Cosmetic change (non-breaking change that doesn't touch code)
None of the above (please explain below)

Checklist:

My code follows the Code Guidelines of this project
My change requires a change to the documentation, either Doxygen or wiki
I have updated the documentation accordingly
I have read the Contributing document
I have added tests to cover my change
All new and existing tests passed

xbmc/cores/VideoPlayer/VideoPlayer.cpp

…m from API

phunkyfish · 2020-04-07T13:38:15Z

I plan to merge this (hopefully tonight) once Jenkins is happy.

FernetMenta · 2020-04-07T13:46:15Z

xbmc/cores/VideoPlayer/VideoPlayer.cpp

@@ -2821,6 +2822,22 @@ void CVideoPlayer::HandleMessages()
        std::shared_ptr<CInputStreamPVRBase> pvrinputstream = std::static_pointer_cast<CInputStreamPVRBase>(m_pInputStream);
        pvrinputstream->Pause(speed == 0);
      }
+
+      // Inform an inputstream addon with a demuxer that an (un)pause has occurred
+      if (m_pInputStream->IsStreamType(DVDSTREAM_TYPE_ADDON) && speed != m_playSpeed &&


this block is nonsense, same applies to the pvr related code above. videoplayer controls reading from demuxer with SetSpeed. if user pauses player and buffers are not full, player continues to read until buffers are full.

Why is it nonsense? The api docs for pausestream simply state that a pause action has occurred. This happens for inputstreams without a demuxer but not for ones with one.

Ok, in hindsight, I agree 👍 this time can be inferred from the demux.

btw, wherever you see m_pInputStream->IsStreamType, you can safely assume that it is outdated crap and following this path does more harm than any good.

FernetMenta · 2020-04-07T13:52:35Z

only used by Inputsream addons for some reason

you should really elaborate on this and make sure the root cause is not in your addon (which is very likely) before changing an API

phunkyfish · 2020-04-07T13:59:36Z

The only inputstream in kodi that uses this API call is the addon inputstream. All the other ignore it.

Is there an issue with supplying a bPaused bool with this?

phunkyfish · 2020-04-07T14:28:32Z

The alternative is create a function only in an Inpustream addon to provide this. But the thinking was why not make the current API more useful.

AlwinEsch · 2020-04-07T14:51:15Z

See it that way, this function is not used anywhere else except for Inputstream. In History it was not immediately possible to find out when this was added.

On my part, it can be a good thing to inform an addon about it, especially if the addon also knows the associated time.

Since the function already exists on the addon interface, it would be good to extend it so that it is always called (and not only if addon has no demux).

phunkyfish · 2020-04-07T19:49:52Z

I will revert the PauseTime changes and replace them with a comment stating that PauseStream is only ever called for an inputstream without it's own demuxer.

Thanks for the guidance again @FernetMenta and sorry for the noise.

phunkyfish · 2020-04-07T22:04:47Z

Will merge this much smaller PR in the morning ;) (Hopefully)

FernetMenta · 2020-04-08T08:17:41Z

See it that way, this function is not used anywhere else except for Inputstream. In History it was not immediately possible to find out when this was added.

I suggest to drop this function from the api. It is outdated and not needed. Inputstream addon started as a clone of pvr and this function was copied though not having been required.

phunkyfish · 2020-04-08T08:58:26Z

I will do a follow up PR removing this function later today. I will need to preserve the functionality in the inputstream addon though as it is used there.

Remove unused Inputstream addon functions and increase max props

phunkyfish added Type: Improvement non-breaking change which improves existing functionality Component: Video API change: Binary add-ons v19 Matrix Type: Breaking change fix or feature that will cause existing functionality to change labels Apr 5, 2020

phunkyfish requested review from AlwinEsch and peak3d April 5, 2020 19:55

phunkyfish mentioned this pull request Apr 5, 2020

Live pause - Matrix API change xbmc/inputstream.ffmpegdirect#27

Merged

phunkyfish added this to the Matrix 19.0-alpha 1 milestone Apr 5, 2020

AlwinEsch reviewed Apr 5, 2020

View reviewed changes

xbmc/cores/VideoPlayer/VideoPlayer.cpp Outdated Show resolved Hide resolved

phunkyfish force-pushed the pause-props branch 2 times, most recently from 0d45cfa to 6d116de Compare April 5, 2020 22:58

[inputstream] remove unused functions CanPauseStream and CanSeekStrea…

31ccf11

…m from API

phunkyfish force-pushed the pause-props branch from 6d116de to 6d018e5 Compare April 6, 2020 10:34

phunkyfish requested a review from ksooo as a code owner April 6, 2020 10:34

phunkyfish force-pushed the pause-props branch from 6d018e5 to 3f28d36 Compare April 6, 2020 13:21

This was referenced Apr 6, 2020

Matrix Inputstream API change to v2.1.0 - Adaptive version 2.5.5 xbmc/inputstream.adaptive#414

Merged

Inputstream API change to v2.1.0 - RTMP version 3.0.2 xbmc/inputstream.rtmp#63

Merged

phunkyfish force-pushed the pause-props branch from 3f28d36 to bb55340 Compare April 7, 2020 12:51

AlwinEsch approved these changes Apr 7, 2020

View reviewed changes

FernetMenta reviewed Apr 7, 2020

View reviewed changes

phunkyfish added 3 commits April 7, 2020 22:05

[inputstream] Update comment on PauseStream() to clarify purpose

999508d

[inputstream] increase max stream properties to 30

a8a2144

[inputstream] bump API to v2.1.0

f9e86b0

phunkyfish force-pushed the pause-props branch from bb55340 to f9e86b0 Compare April 7, 2020 21:14

phunkyfish changed the title ~~Remove unused Inputstream addon functions, increase max props and PauseStream() call in Video Player~~ Remove unused Inputstream addon functions and increase max props Apr 7, 2020

phunkyfish merged commit 13cecb9 into xbmc:master Apr 8, 2020

phunkyfish deleted the pause-props branch April 8, 2020 11:02

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 8, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

5003985

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request May 5, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

2ef51da

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

47f2d39

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

83cea97

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

7fdbf89

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

92fbf3a

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

bbc37b0

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

51ae258

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

d321dde

Remove unused Inputstream addon functions and increase max props

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020

Merge pull request xbmc#17620 from phunkyfish/pause-props

aa2f8ef

Remove unused Inputstream addon functions and increase max props

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unused Inputstream addon functions and increase max props #17620

Remove unused Inputstream addon functions and increase max props #17620

phunkyfish commented Apr 5, 2020 •

edited

phunkyfish commented Apr 7, 2020

FernetMenta Apr 7, 2020

phunkyfish Apr 7, 2020

phunkyfish Apr 7, 2020

FernetMenta Apr 8, 2020

FernetMenta commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

AlwinEsch commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

FernetMenta commented Apr 8, 2020

phunkyfish commented Apr 8, 2020 •

edited

Remove unused Inputstream addon functions and increase max props #17620

Remove unused Inputstream addon functions and increase max props #17620

Conversation

phunkyfish commented Apr 5, 2020 • edited

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

Checklist:

phunkyfish commented Apr 7, 2020

FernetMenta Apr 7, 2020

Choose a reason for hiding this comment

phunkyfish Apr 7, 2020

Choose a reason for hiding this comment

phunkyfish Apr 7, 2020

Choose a reason for hiding this comment

FernetMenta Apr 8, 2020

Choose a reason for hiding this comment

FernetMenta commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

AlwinEsch commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

phunkyfish commented Apr 7, 2020

FernetMenta commented Apr 8, 2020

phunkyfish commented Apr 8, 2020 • edited

phunkyfish commented Apr 5, 2020 •

edited

phunkyfish commented Apr 8, 2020 •

edited