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

[PVR] Feature: Make instant recording behavior configurable #9847

Merged
merged 4 commits into from Jun 1, 2016

Conversation

Projects
None yet
5 participants
@ksooo
Copy link
Member

commented May 20, 2016

This feature has been requested in at least two different forum threads.

Currently, when pressing the "record" button in Live TV OSD, on the remote control or on keyboard a fixed length recording starting "now", ending after the time set as "Instant timer duration" setting value will be scheduled.

People want more flexibility here, especially the possibility to let instant recording record the currently playing show.

This PR implements configurable instant recording behavior.

First, there is a new settings value in PVR->Recording, "Instant recordig action":

screenshot000

Three values are available:

screenshot001

  • [Record current show] will record the current show from "now" to the end of the show. If no TV guide data is currently available a fixed length recording starting "now", with the value set for "Instant recording duration" will be scheduled.
  • [Record for a fixed period of time] will schedule a fixed length recording starting "now", with the value set for "Instant recording duration".
  • [Ask what to do] will open a dialogue containing different recording actions to choose from, like "Record current show", "Record next show" and some fixed duration recordings.

The first two options will immediately start recording without any user interaction, the third option will open a dialog with a couple of choices:

screenshot003

The options currently implemented for the dialog as well as the wording are only a suggestion and can easily be changed.

@da-anda we discussed this on last Devcon
@Jalle19, @xhaggi mind taking a look at the code

@ksooo ksooo force-pushed the ksooo:pvr-improve-instant-timer branch from 089c008 to e3bdd75 May 20, 2016

#. Label for "Instant recording action" dialog settings value
#: xbmc/pvr/PVRMananger.cpp
msgctxt "#19092"
msgid "Record next show (%s)"

This comment has been minimized.

Copy link
@da-anda

da-anda May 21, 2016

Member

Maybe Record upcoming show? But a native should say what's more appropriate. @jjd-uk?

This comment has been minimized.

Copy link
@ksooo

ksooo May 21, 2016

Author Member

"upcoming" denotes any show somewhere in the future, not precisely the very next in the timeline, I'd say. So, "next" should fit better here. But ofc I'm not a native English speaker.

This comment has been minimized.

Copy link
@jjd-uk

jjd-uk May 21, 2016

Member

I'd say stick with the strings ksooo has, seem entirely aporopiate to me.

This comment has been minimized.

Copy link
@Jalle19

Jalle19 May 21, 2016

Member

We use "next" elsewhere too so it seems logical to use it here as well.

@ksooo ksooo force-pushed the ksooo:pvr-improve-instant-timer branch 4 times, most recently from 6c2c329 to eb30cb3 May 23, 2016

@ksooo ksooo force-pushed the ksooo:pvr-improve-instant-timer branch 2 times, most recently from 8599517 to 4c0869f May 30, 2016

@ksooo

This comment has been minimized.

Copy link
Member Author

commented May 31, 2016

If nobody objects I will merge this tomorrow.

{
enum PVRRECORD_INSTANTRECORDACTION
{
PVRRECORD_INSTANTRECORDACTION_NONE = -1,

This comment has been minimized.

Copy link
@Jalle19

Jalle19 May 31, 2016

Member

Couldn't these names be shortened to e.g. NONE? The "namespace" of the enum is already made clear through the enum's name (PVRRECORD_INSTANTRECORDACTION).

This comment has been minimized.

Copy link
@ksooo

ksooo May 31, 2016

Author Member

Fair enough. Let's see whether this works without creating any preprocessor problems.

newTimer->UpdateSummary();
newTimer->m_strSummary = StringUtils::Format(g_localizeStrings.Get(19093).c_str(), newTimer->Summary().c_str());

CDateTime startTime(0); // special start time value that denotes an instant timer.

This comment has been minimized.

Copy link
@Jalle19

Jalle19 May 31, 2016

Member

Possible candidate for a const int/#define?

This comment has been minimized.

Copy link
@ksooo

ksooo May 31, 2016

Author Member

Yep. Good point.

@Jalle19

This comment has been minimized.

Copy link
Member

commented May 31, 2016

Sorry for the late review, nothing critical found though.

@ksooo

This comment has been minimized.

Copy link
Member Author

commented May 31, 2016

Thanks @Jalle19. I thought you already did a review.

@Jalle19

This comment has been minimized.

Copy link
Member

commented May 31, 2016

Only did it half way the first time :-P

@ksooo ksooo force-pushed the ksooo:pvr-improve-instant-timer branch from 7ac5664 to ac5193e May 31, 2016

@ksooo

This comment has been minimized.

Copy link
Member Author

commented May 31, 2016

jenkins build this please

@ksooo

This comment has been minimized.

Copy link
Member Author

commented Jun 1, 2016

Changes requested by @Jalle19 are in.
Rebased.
Jenkins is happy.

@ksooo ksooo merged commit ecea559 into xbmc:master Jun 1, 2016

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
default Merged build finished.
Details

@ksooo ksooo deleted the ksooo:pvr-improve-instant-timer branch Jun 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.