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

Fix playback when skipping an in progress recording to real-time #413

Merged
merged 1 commit into from
Apr 18, 2019
Merged

Fix playback when skipping an in progress recording to real-time #413

merged 1 commit into from
Apr 18, 2019

Conversation

jahutchi
Copy link
Contributor

@jahutchi jahutchi commented Apr 12, 2019

Description

This PR resolves issue #379: HTSPVFS and in progress recordings

Motivation and Context

Currently, if you skip right to the end of an in progress recording, then the playback completely chokes; i.e. both the audio and video break up badly, the video freezes, and the audio makes popping noises.

On debugging, I found that the main issue was that Tvheadend can briefly return 0 bytes when playing an in-progress recording towards EOF; the demuxer expects at least 1 bytes to be returned.

My PR resolves this issue by adjusting HTSPVFS::Read() to retry when 0 bytes are returned for an in-progress recording.

After fixing this problem, the other issue I found was that Kodi would buffer for around 10 seconds when skipping fwd to real-time, before eventually resuming normal playback. After some debugging and research I landed on the IsRealTimeStream API call. This PR returns true for IsRealTimeStream when playback is within the last 10 seconds of an in-progress recording.

How Has This Been Tested?

My testing was over a Gigabit Ethernet using both HD and SD channels.

Backend: TVheadend (v4.2.8) running on a 10yr old HTPC (Acer Revo 3700) running Debian Stretch
Kodi: Running on a Vero4K+ using OSMC Leia nightlies with pvr.hts v4.4.16 (+ this patch).

In this setup, I have conducted several tests as follows:

  • Watched a normal, completed recording from start to finish to ensure no regressions were introduced for normal recordings. Also added extra debug logging to show the value of IsRealTimeStream, and could see it was always set to false as it always has been.
  • Played an in-progress recording and skipped way beyond real-time playback. This caused the seek to 'snap back' to the end of file i.e. "Real-time" playback. Playback started almost immediately (buffering circle appeared for only a split second), and playback was fine thereafter.
  • Played an in-progress recording (1hr long) and skipped to real-time playback. Then watched the recording right to the end. No stutters or playback issues were observed, and when the show finished the playback ended immediately.
  • With extra debugging in place, I monitored the value of IsRealTimeStream on an in-progress recording. I performed several small skip operations, and IsRealTimeStream was only set to true when nearing the end of the file (within last 10 seconds).
  • Played LiveTV and also tried skipping backwards and forwards in the timeshift buffer to ensure no regressions were introduced in this area.
  • Repeated the tests of skipping an in-progress recording to real-time playback using different chunk sizes. I have tried with both the minimum (4KB) and maximum (512KB) and playback was fine in all cases.
  • Tried starting a recording and playing it almost immediately after. Playback was fine and I skipped forward (+2-3 seconds) to real-time playback without any problems / buffering /stuttering.

New tests added (16/04):

  • Skip forward to real-time playback, then hit pause for 10 seconds, hit play and watch the logfile to ensure that IsRealTimeStream is set to false
  • Skip forward to real-time playback, hit pause, then hit skip back (10 seconds) followed by play. Watch the logfile to ensure that IsRealTimeStream is set back to false.
  • Pause an in progress recording (non real-time); whilst paused, skip forward to real-time then hit play. Ensure that IsRealTimeStream is set to true.
  • Pause an in progress recording (non real-time); whilst paused, skip forward to real-time, then wait >10 seconds, then hit play. Ensure that IsRealTimeStream is set to false.

@jahutchi jahutchi changed the title 4.4.17: Fix playback when skipping an in progress recording to real-time Fix playback when skipping an in progress recording to real-time Apr 12, 2019
@ksooo
Copy link
Member

ksooo commented Apr 12, 2019

@FernetMenta what is your opinion on this?

src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
@Rechi Rechi changed the base branch from master to Leia April 15, 2019 20:55
@jahutchi
Copy link
Contributor Author

I've completed my testing using the tests outlined above, and all tests above passed successfully - now handles pauses correctly WRT setting IsRealTimeStream :-)

Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minors only.

src/Tvheadend.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
@jahutchi
Copy link
Contributor Author

@ksooo care to take another look?

I don't expect these changes to have caused any regressions, but please don't merge until I've retested (tonight) just to be on the safe side.

@ksooo
Copy link
Member

ksooo commented Apr 18, 2019

You have not (force-)pushed the changes so far. Cannot re-review.

@jahutchi
Copy link
Contributor Author

You have not (force-)pushed the changes so far. Cannot re-review.

My apologies.... trying again

@jahutchi
Copy link
Contributor Author

Now ready for re-review ;-)

Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost done, I'd say.

src/Tvheadend.h Outdated Show resolved Hide resolved
src/Tvheadend.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
@jahutchi
Copy link
Contributor Author

jahutchi commented Apr 18, 2019

almost done, I'd say.

Latest changes pushed... still need to test

Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Tell me, when you're done with your tests.

src/tvheadend/HTSPVFS.cpp Outdated Show resolved Hide resolved
@jahutchi
Copy link
Contributor Author

@ksooo I'm done with my testing, all working as expected.

@ksooo
Copy link
Member

ksooo commented Apr 18, 2019

Here we go... thanks for your contribution.

@@ -309,6 +312,12 @@ void SetSpeed(int speed)
tvh->DemuxSpeed(speed);
}

void PauseStream(bool paused)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using this function is wrong. you have to recalculate the conditions in read because player can run at lower speed, i.e. 0.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants