LinuxTV Patchwork [2.3.9] Setting a mark is sluggish

login
register
mail settings
Submitter Klaus Schmidinger
Date April 2, 2018, 4:40 p.m.
Message ID <6c3d6e6b-d766-e3c1-a852-227e2b5dbde6@tvdr.de>
Download mbox | patch
Permalink /patch/48325/
State New
Headers show

Comments

Klaus Schmidinger - April 2, 2018, 4:40 p.m.
On 02.04.2018 14:20, Oliver Endriss wrote:
> Am Montag, den 02.04.2018, 12:28 +0200 schrieb Klaus Schmidinger:
>> On 01.04.2018 19:01, Oliver Endriss wrote:
>> > ...
>> >> >> >> Does it make a difference whether the progress display is active or not
>> >> >> >> when you set the mark?
>> >> > 
>> >> > If the progress bar is off, and you set a mark, progress bar and
>> >> > mark show up immediately. -> No problem.
>> >> 
>> >> ...
>> >> > Could it be that this action is triggered _before_ the mark has been
>> >> > written to the OSD buffer? In this case, the OSD would be displayed
>> >> > without the mark, and the mark would be displayed during the next
>> >> > cycle, i.e. one second later.
>> >> 
>> >> I doubt that.
>> 
>> Well, meanwhile I think you were right here.
>> In cReplayControl::MarkToggle() there is
>> 
>>    lastCurrent = -1; // triggers redisplay
>> 
>> which it used to do until the switch to GetFrameNumber() ;-).
>> 
>> Now this has no immediate effect any more, and that may explain
>> the sluggishness you observe.
>> 
>> Please try this:
>> 
>> --- menu.c      2018/03/24 11:43:40     4.70
>> +++ menu.c      2018/04/02 10:07:05
>> @@ -5728,7 +5728,7 @@
>>   bool cReplayControl::ShowProgress(bool Initial)
>>   {
>>     int Current, Total;
>> -  if (Initial || lastSpeed != -1 || time(NULL) - lastProgressUpdate >= 1) {
>> +  if (Initial || lastCurrent < 0 || lastSpeed != -1 || time(NULL) - lastProgressUpdate >= 1) {
>>        if (GetFrameNumber(Current, Total) && Total > 0) {
>>           if (!visible) {
>>              displayReplay = Skins.Current()->DisplayReplay(modeOnly);
>> 
>> > I am pretty sure that something is wrong.
>> > With the following (ugly) patch, the problem is gone:
>> > ...
>> > All it does is executing the code in 'if (Initial...)' one more time,
>> > after lastProgressUpdate has been set to 0.
>> 
>> Thanks for pointing this out. This triggered my idea with lastCurrent above.
> 
> Thanks, your patch fixed the issue.

Taking a step back and considering that GetFrameNumber() was supposed to
be a drop-in replacement for GetIndex(), just returning the exact number
of the currently replayed frame and not just the index into the 'index'
file (which, apart from I-frames, is typically different) I revoked the
whole change and simply replaced GetIndex() with GetFrameNumber(). That
resulted in the sluggish progress display on the Raspberry Pi I reported
earlier. I then found that this was caused by the extra Flush() calls in
cReplayControl::ShowProgress(). So I removed them and now everything runs
smoothly on the RasPi and also on softhddevice.
So please try the attached patch instead of the previous one, and especially
check whether it still works on the dvbsddevice. This should hopefully also
fix the "jumping progress display".

There is, apparently, still a problem on the RasPi, where once you set
a mark, a subsequent "play" doesn't take effect until a couple of seconds
later. However, since this doesn't occur with softhddevice, I assume this
is a RasPi specific problem. I'll discuss this with Thomas separately.

Klaus
Oliver Endriss - April 2, 2018, 11:25 p.m.
Am Montag, den 02.04.2018, 18:40 +0200 schrieb Klaus Schmidinger:
> On 02.04.2018 14:20, Oliver Endriss wrote:
> > Am Montag, den 02.04.2018, 12:28 +0200 schrieb Klaus Schmidinger:
> >> On 01.04.2018 19:01, Oliver Endriss wrote:
> >> > ...
> >> >> >> >> Does it make a difference whether the progress display is active or not
> >> >> >> >> when you set the mark?
> >> >> > 
> >> >> > If the progress bar is off, and you set a mark, progress bar and
> >> >> > mark show up immediately. -> No problem.
> >> >> 
> >> >> ...
> >> >> > Could it be that this action is triggered _before_ the mark has been
> >> >> > written to the OSD buffer? In this case, the OSD would be displayed
> >> >> > without the mark, and the mark would be displayed during the next
> >> >> > cycle, i.e. one second later.
> >> >> 
> >> >> I doubt that.
> >> 
> >> Well, meanwhile I think you were right here.
> >> In cReplayControl::MarkToggle() there is
> >> 
> >>    lastCurrent = -1; // triggers redisplay
> >> 
> >> which it used to do until the switch to GetFrameNumber() ;-).
> >> 
> >> Now this has no immediate effect any more, and that may explain
> >> the sluggishness you observe.
> >> 
> >> Please try this:
> >> 
> >> --- menu.c      2018/03/24 11:43:40     4.70
> >> +++ menu.c      2018/04/02 10:07:05
> >> @@ -5728,7 +5728,7 @@
> >>   bool cReplayControl::ShowProgress(bool Initial)
> >>   {
> >>     int Current, Total;
> >> -  if (Initial || lastSpeed != -1 || time(NULL) - lastProgressUpdate >= 1) {
> >> +  if (Initial || lastCurrent < 0 || lastSpeed != -1 || time(NULL) - lastProgressUpdate >= 1) {
> >>        if (GetFrameNumber(Current, Total) && Total > 0) {
> >>           if (!visible) {
> >>              displayReplay = Skins.Current()->DisplayReplay(modeOnly);
> >> 
> >> > I am pretty sure that something is wrong.
> >> > With the following (ugly) patch, the problem is gone:
> >> > ...
> >> > All it does is executing the code in 'if (Initial...)' one more time,
> >> > after lastProgressUpdate has been set to 0.
> >> 
> >> Thanks for pointing this out. This triggered my idea with lastCurrent above.
> > 
> > Thanks, your patch fixed the issue.
> 
> Taking a step back and considering that GetFrameNumber() was supposed to
> be a drop-in replacement for GetIndex(), just returning the exact number
> of the currently replayed frame and not just the index into the 'index'
> file (which, apart from I-frames, is typically different) I revoked the
> whole change and simply replaced GetIndex() with GetFrameNumber(). That
> resulted in the sluggish progress display on the Raspberry Pi I reported
> earlier. I then found that this was caused by the extra Flush() calls in
> cReplayControl::ShowProgress(). So I removed them and now everything runs
> smoothly on the RasPi and also on softhddevice.
> So please try the attached patch instead of the previous one, and especially
> check whether it still works on the dvbsddevice. This should hopefully also
> fix the "jumping progress display".

Works fine with HD/SD/radio recordings. Tested with dvbhddevice
and dvbsddevice. Both problems are fixed. Thank you.

> There is, apparently, still a problem on the RasPi, where once you set
> a mark, a subsequent "play" doesn't take effect until a couple of seconds
> later. However, since this doesn't occur with softhddevice, I assume this
> is a RasPi specific problem. I'll discuss this with Thomas separately.

This does not occur with the FF cards.

Oliver

Patch

--- menu.h	2018/02/01 15:35:48	4.6
+++ menu.h	2018/04/02 13:41:49
@@ -300,7 +300,6 @@ 
   bool lastPlay, lastForward;
   int lastSpeed;
   time_t timeoutShow;
-  time_t lastProgressUpdate;
   bool timeSearchActive, timeSearchHide;
   int timeSearchTime, timeSearchPos;
   void TimeSearchDisplay(void);
--- menu.c	2018/03/24 11:43:40	4.70
+++ menu.c	2018/04/02 13:41:39
@@ -5564,7 +5564,6 @@ 
   lastPlay = lastForward = false;
   lastSpeed = -2; // an invalid value
   timeoutShow = 0;
-  lastProgressUpdate = 0;
   timeSearchActive = false;
   cRecording Recording(fileName);
   cStatus::MsgReplaying(this, Recording.Name(), Recording.FileName(), true);
@@ -5728,43 +5727,36 @@ 
 bool cReplayControl::ShowProgress(bool Initial)
 {
   int Current, Total;
-  if (Initial || lastSpeed != -1 || time(NULL) - lastProgressUpdate >= 1) {
-     if (GetFrameNumber(Current, Total) && Total > 0) {
-        if (!visible) {
-           displayReplay = Skins.Current()->DisplayReplay(modeOnly);
-           displayReplay->SetMarks(&marks);
-           SetNeedsFastResponse(true);
-           visible = true;
-           }
-        if (Initial) {
-           if (*fileName) {
-              LOCK_RECORDINGS_READ;
-              if (const cRecording *Recording = Recordings->GetByName(fileName))
-                 displayReplay->SetRecording(Recording);
-              }
-           lastCurrent = lastTotal = -1;
+  if (GetFrameNumber(Current, Total) && Total > 0) {
+     if (!visible) {
+        displayReplay = Skins.Current()->DisplayReplay(modeOnly);
+        displayReplay->SetMarks(&marks);
+        SetNeedsFastResponse(true);
+        visible = true;
+        }
+     if (Initial) {
+        if (*fileName) {
+           LOCK_RECORDINGS_READ;
+           if (const cRecording *Recording = Recordings->GetByName(fileName))
+              displayReplay->SetRecording(Recording);
            }
-        if (Current != lastCurrent || Total != lastTotal) {
-           time(&lastProgressUpdate);
-           if (Setup.ShowRemainingTime || Total != lastTotal) {
-              int Index = Total;
-              if (Setup.ShowRemainingTime)
-                 Index = Current - Index;
-              displayReplay->SetTotal(IndexToHMSF(Index, false, FramesPerSecond()));
-              if (!Initial)
-                 displayReplay->Flush();
-              }
-           displayReplay->SetProgress(Current, Total);
-           if (!Initial)
-              displayReplay->Flush();
-           displayReplay->SetCurrent(IndexToHMSF(Current, displayFrames, FramesPerSecond()));
-           displayReplay->Flush();
-           lastCurrent = Current;
+        lastCurrent = lastTotal = -1;
+        }
+     if (Current != lastCurrent || Total != lastTotal) {
+        if (Setup.ShowRemainingTime || Total != lastTotal) {
+           int Index = Total;
+           if (Setup.ShowRemainingTime)
+              Index = Current - Index;
+           displayReplay->SetTotal(IndexToHMSF(Index, false, FramesPerSecond()));
            }
-        lastTotal = Total;
-        ShowMode();
-        return true;
+        displayReplay->SetProgress(Current, Total);
+        displayReplay->SetCurrent(IndexToHMSF(Current, displayFrames, FramesPerSecond()));
+        displayReplay->Flush();
+        lastCurrent = Current;
         }
+     lastTotal = Total;
+     ShowMode();
+     return true;
      }
   return false;
 }
@@ -6007,8 +5999,6 @@ 
      return osEnd;
   if (Key == kNone && !marksModified)
      marks.Update();
-  if (Key != kNone)
-     lastProgressUpdate = 0;
   if (visible) {
      if (timeoutShow && time(NULL) > timeoutShow) {
         Hide();

Privacy Policy