v32 upgrade - exit menu changed?

[marcjw]

• Platform:

• 5.13.0-39-generic en-gb updates for 0.26-fixes #44~20.04.1-Ubuntu SMP Thu Mar 24 16:43:35 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

• MythTV version:
  32

• Package version:

• Component:

What steps will reproduce the bug?

watch a recorded TV show and then exit

I just completed a v31 -> v32 upgrade this morning for all BE and FE on a bunch of Ubuntu 20.02 machines. Everything seems to have gone well. But I noticed something I don't like. The exit menu from a TV recording now doesn't have a "exit but don't save" option. I'm pretty sure v31 had that. I want to be able to watch a recording and then be able to have my wife watch the same recording from the beginning, not the bookmark where I exited. Without that menu option I'm forced to go a different place a delete that bookmark.

How often does it reproduce? Is there a required condition?

every time

What is the expected behaviour?

The exit menu should have an option to exit without saving (bookmark). I recall that v31 had this.

What do you see instead?

Additional information

[linuxdude42]

This was part of an intentional reworking of the bookmark. See #331 for more information.

-- from an email to mythtv-users --
MythTV formerly used the recording bookmark for both 1) a user bookmark
and 2) to remember its position in the recording when you stop play.
These have been split into separate items. The bookmark now remains
untouched until you the user set or clear it, and the last played
position is always remembered.

When you start a recording from the "Watch Recordings" page, if you
choose a recording and hit enter MythTV will begin play from 1) the
last play position (if set), 2) the user bookmark (if set), or 3) the
beginning. If you choose a recording and hit Menu then MythTV will let
you select which of the available starting points to use. These
options for starting play aren't new, they were always present and
worked with the bookmark and the beginning. What's new is that these
options now include the last played position.

If you are using one of the MythCenter themes, the "Watch Recordings"
page now shows an indicator of what percentage of a recording has been
watched. This is based on the last play position.

[paul-h]

In an idea world we would extend the multi-user support that stuartm started and make those settings per user rather than global settings.

[kmdewaal]

@linuxdude42 you do describe the way it now works in v32 but you do not address the issue raised.

In v31 the exit menu does have an option to "exit wiithout saving" and this then does not update the bookmark or the last played position (which is the same in v31). The next playback then starts at the beginning or at the bookmark position.

In v32 the exit menu does not have this option anymore and there is no way to exit without updating the last played position. This then means that the only way to start playback from the beginning is to use the menu option as described.

@paul-h having multi-user support would definitely open up new possibilities. But in this case it is just feature that is removed.

[kmdewaal]

This is the exit menu of v31.

This is the exit menu of v32.

[kmdewaal]

There is also a change in behavior in the "Action on Playback Exit". On v31 the configuration menu looks like this:

Here the choices "Exit Without Saving" and "Save on Exit" do what is promised; when "Exit Without Saving" is selected the next playback does start at the same position again and when "Save on Exit" is selected the next playback continues at the last play position.

On v32 the "Action on Playback Exit" looks like this:

Here the "Save on Exit" choice is removed and only the "Exit WIthout Saving" can be selected if you do not want a prompt.
However, when "Exit Without Saving" is selected the next playback continues at the last play position which was in v31 the behavior of "Save on Exit" selection.

I think that the configuration options "Save on Exit" and "Exit Without Saving" and the corresponding behavior of v31 is correct.

[gigem]

For the record, I'm in favor of doing away with this mostly useless, menu option just for the code simplification. It's easy enough to clear the last played postion after exit and/or restart playback from the beginning.

[marcjw]

gigem,
Sure, why make it easier for a user, right? I mean, it's not like spouses or others less inclined ever use the product, right?

[gigem]

It's easier by providing a simpler interface and a simpler and more reliable program because it doesn't have to carry along a lot of legacy support of dubious value.

[ulmus-scott]

Since there is an optional prompt on playback exit, is there a way to optionally make the prompt on where to start playback always show if there is a bookmark or saved position? (When playing a recording via enter not → → (two keypresses) to get to the menu.)

[marcjw]

Of course they can; it's just code. But I've already been told they won't. They would rather save a few lines of code at the expense of usability.

[BobEntwhistle]

This new behaviour has caused a good deal of annoyance in my household. How can I revert the behaviour back to that of v31?
Is there a flag I can set to stop my frontend from behaving this way?
I don't want to restart playback at the point that I last exited. Ever.

[royboy626]

I hate to "me too"... but echoing BobE
This new behaviour has caused a good deal of annoyance in my household. How can I revert the behaviour back to that of v31?
Is there a flag I can set to stop my frontend from behaving this way?
I don't want to restart playback at the point that I last exited. Ever.

[kmdewaal]

20220628-clearposonexit.patch.gz

Find attached a patch for fixes/32 that restores the "Exit Without Saving" button of fixes/31 in the playback exit menu.
In fixes/31 the bookmark representing the last_played_position is only updated on playback exit. Or not, when so chosen.
In fixes/32 the last_played_position is continuously updated so there is really no way to do a real "Exit Without Saving" because the last_played_position has been saved already.
This patch implements the "Exit Without Saving" action by clearing the last_played_position on playback exit. This insures that the next playback starts at the beginning or at the bookmark, when present.

@linuxdude42 It may not be what you or I want to use in our houses but apparently some users do appreciate this.
@gigem Yes it is a few lines of code extra but I think the added complexity is minimal.
@marcjw and others, this patch can be tested when you compile fixes/32 from source.

[marcjw]

Thanks, kmdewaal.
But I don't think that patch will do much good for those of us Ubuntu users who use the Focal PPA for our fixes/32. At least I don't know how.

[kmdewaal]

For easy reading of the patch, this is it in readable form:

diff --git a/mythtv/libs/libmythtv/tv_play.cpp b/mythtv/libs/libmythtv/tv_play.cpp
index 2803837fb2..a420891fdd 100644
--- a/mythtv/libs/libmythtv/tv_play.cpp
+++ b/mythtv/libs/libmythtv/tv_play.cpp
@@ -2717,13 +2717,19 @@ void TV::PrepToSwitchToRecordedProgram(const ProgramInfo &ProgInfo)
 void TV::PrepareToExitPlayer(int Line)
 {
     m_playerContext.LockDeletePlayer(__FILE__, Line);
-    if (m_savePosOnExit && m_player && m_playerContext.m_playingInfo)
+    if ((m_savePosOnExit || m_clearPosOnExit) && m_player && m_playerContext.m_playingInfo)
     {
         // Clear last play position when we're at the end of a recording.
         // unless the recording is in-progress.
         bool at_end = !StateIsRecording(m_playerContext.GetState()) &&
                 (GetEndOfRecording() || m_playerContext.m_player->IsNearEnd());
 
+        // Clear last play position on exit when the user requested this
+        if (m_clearPosOnExit)
+        {
+            at_end = true;
+        }
+
         // Clear/Save play position without notification
         // The change must be broadcast when file is no longer in use
         // to update previews, ie. with the MarkNotInUse notification
@@ -9811,6 +9817,8 @@ void TV::ShowOSDStopWatchingRecording()
 
     dialog.m_buttons.push_back({tr("Exit %1").arg(videotype), ACTION_STOP});
 
+    dialog.m_buttons.push_back({tr("Exit Without Saving"), "DIALOG_VIDEOEXIT_CLEARLASTPLAYEDPOSITION_0"});
+
     if (IsDeleteAllowed())
         dialog.m_buttons.push_back({tr("Delete this recording"), "DIALOG_VIDEOEXIT_CONFIRMDELETE_0"});
 
@@ -9958,6 +9966,12 @@ bool TV::HandleOSDVideoExit(const QString& Action)
     {
         DoTogglePause(true);
     }
+    else if (Action == "CLEARLASTPLAYEDPOSITION")
+    {
+        m_clearPosOnExit = true;
+        PrepareToExitPlayer(__LINE__);
+        SetExitPlayer(true, true);
+    }
 
     return hide;
 }
diff --git a/mythtv/libs/libmythtv/tv_play.h b/mythtv/libs/libmythtv/tv_play.h
index 6bc0acc587..015e9f995f 100644
--- a/mythtv/libs/libmythtv/tv_play.h
+++ b/mythtv/libs/libmythtv/tv_play.h
@@ -528,7 +528,7 @@ class MTV_PUBLIC TV : public TVPlaybackState, public MythTVMenuItemDisplayer, pu
     uint              m_vbimode {VBIMode::None};
     uint              m_switchToInputId {0};
 
-    /// True if the user told MythTV to stop plaback. If this is false
+    /// True if the user told MythTV to stop playback. If this is false
     /// when we exit the player, we display an error screen.
     mutable bool      m_wantsToQuit {true};
     bool              m_stretchAdjustment {false}; ///< True if time stretch is turned on
@@ -543,6 +543,7 @@ class MTV_PUBLIC TV : public TVPlaybackState, public MythTVMenuItemDisplayer, pu
     bool              m_doSmartForward {false};
     bool              m_queuedTranscode {false};
     bool              m_savePosOnExit {false};  ///< False until first timer event
+    bool              m_clearPosOnExit {false}; ///< False unless requested by user on playback exit
     /// Picture attribute type to modify.
     PictureAdjustType m_adjustingPicture {kAdjustingPicture_None};
     /// Picture attribute to modify (on arrow left or right)

[kmdewaal]

@marcjw

Thanks, kmdewaal. But I don't think that patch will do much good for those of us Ubuntu users who use the Focal PPA for our fixes/32. At least I don't know how.

Those building from source can give it a try and those who read source code can also give comment. Next step is to put this in master and if that performs as expected without regressions then it can be put in fixes/32. Then, in mysterious ways, packages will be updated and everybody can update their installation from the packages.

[kmdewaal]

New&Improved patch, now also adds a Clear last played position and exit playback setup configuration choice.
I think this now completely implements the option to not save the last played position.
The last played position is actually maintained all the time when playing; this option makes it now possible to clear it on playback exit. How the bookmarks work should not be affected.
I will do a bit more testing before committing.

20220629-clearposonexit.patch.txt

[ulmus-scott]

Using the new option appears to restore the behavior to that of v31.

I still would also like an option that creates a popup asking where to start playback of the recording when selecting a recording with Enter (if there is a bookmark or a previous played position) instead of having to press → →, but that could be done separately.

[royboy626]

ulmus-scott: Does pressing the 'm' key when a recordings is selected instead of [Enter] meet your needs? No need to reply. I am running mythtv-frontend-32.0-1.36.20220605git7077a824d2.fc36.x86_64 (Fedora/RPMFusion)

kmdewaal: Thank you so much for your work. I will test the fixes/32 branch and your latest patch should you commit it to the tree.

[ulmus-scott]

ulmus-scott: Does pressing the 'm' key when a recordings is selected instead of [Enter] meet your needs? No need to reply. I am running mythtv-frontend-32.0-1.36.20220605git7077a824d2.fc36.x86_64 (Fedora/RPMFusion)

That brings up the same menu as →, "Recording Options". What I want is the "Play Options" menu with a single key press.

[royboy626]

In limited testing, "Clear last played position and exit" seems to work as intended. Thank you for your time. I will be using this option. I believe one additional change would increase flexibility in dealing with playback/bookmarks, and would restore the behavior of fixes/31. My systems were set to clear (manual) bookmark on playback/resume of a recording in fixes/31. I don't think this is possible now.

This would potentially be a check box ("Action on Playback Resume"), outside the 'Action on Playback Exit' radio button pop-up (so I could set "Clear last played position and exit" as well), to offer maximum flexibility to the user.

One scenario using "Clear last played position and exit": A manual bookmark is set. Later the recording is resumed, and one may or may not notice it is not the beginning of the recording (depending on recording content: a partial recording of sports is an example), so the bookmark must be toggled to check the bookmark status. The combination of clearing manual bookmarks on resume and last played position on exit means a resumed recording is in a clean state regarding bookmarks/playback position. If these two options were available and set. I know the state of all my recordings, and if I wish to add a bookmark, I have control.

This may not be considered worthy of further coding and time. If so, I will live with the current commit and adjust if necessary.

Per-user and multiple bookmarks are indeed something that would be 'nice to have', but that is for another day.

For these patches/commits to more quickly reach users, they should be ported to the fixes/32 branch.

[kmdewaal]

Thanks for testing. There has some time passed in master and no regression have been reported so this is a good moment to put it in fixes/32. This fix is about the new "last played position" and does not change the way bookmarks are done.
However, in fixes/31 there is the option "Clear bookmark on playback" which is not present anymore in fixes/32.
Is that the option you are referring to?

[royboy626]

Yes, absolutely. "Clear bookmark on playback" would be nice to have (at least in one person's opinion). Thanks for going the extra mile and checking on this. As to if it gets restored to fixes/32, obviously your call.

[kmdewaal]

I did have a look at it but the code behind the "Clear bookmark on playback" in fixes/31 is rather complicated. It does clear an existing bookmark on program exit but only if it is near the end and if the "Save on Exit" option is selected. This is all about using the bookmark to save and clear the last played position and not about using bookmarks as bookmarks.
The current implementation treats the last played position and bookmarks completely independent and I think it is better this way. So the "Clear bookmark on playback" will not be resurrected.
Thanks for the reporting and the testing, and now closing this ticket.

[kmdewaal]

Reverted in master by commit 2f45323 and in fixes/32 by commit 6b2d23d because of reported regression in handling the last played position.

[kmdewaal]

I've just now pushed a new commit to master with the same functionality but hopefully without the regression that necessitated the revert of the previous commit to master. As before, if this has been in master for a while it will be ported back to fixes/32.

[kmdewaal]

The new implementation is identical to the previous one other than that the numeric value of the option "Clear last played position and exit" is changed. The consequence is that after an upgrade the configuration will be reset to "Just exit" so the "Clear last played position" configuration option must be selected again.