Opened 5 years ago

Closed 5 years ago

#11141 closed Patch - Bug Fix (fixed)

Experimental commerciall flagging options D2 goes into finite loop on frame 0 taking 100% CPU

Reported by: Wayne McDougall <waynemcdougall@…> Owned by: stichnot
Priority: minor Milestone: 0.27
Component: MythTV - Mythcommflag Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no


The current code just keeps requesting frame 0.

Attachments (2)

D2.patch (602 bytes) - added by Wayne McDougall <waynemcdougall@…> 5 years ago.
D2_v2.patch (2.8 KB) - added by Wayne McDougall <waynemcdougall@…> 5 years ago.

Download all attachments as: .zip

Change History (9)

Changed 5 years ago by Wayne McDougall <waynemcdougall@…>

comment:1 Changed 5 years ago by Wayne McDougall <waynemcdougall@…>

I think this bug was introduced as an unintended side effect of this commit Remove the "Seek to exact frame" setting.:

and specifically this change in mythPlayer.cpp

@@ -4328,7 +4330,7 @@ VideoFrame* MythPlayer::GetRawVideoFrame(long long frameNumber)

 -        DoJumpToFrame(frameNumber, true, true);

 +        DoJumpToFrame(frameNumber, kInaccuracyNone);

comment:2 Changed 5 years ago by stuartm

  • Owner changed from cpinkham to stichnot
  • Status changed from new to assigned

comment:3 Changed 5 years ago by stichnot

  • Milestone changed from unknown to 0.26.1
  • Status changed from assigned to accepted
  • Version changed from Unspecified to Master Head

Changed 5 years ago by Wayne McDougall <waynemcdougall@…>

comment:4 Changed 5 years ago by Wayne McDougall <waynemcdougall@…>

D2.patch does not handle the case where Logo detection wants to skip over 100 frames at a time.

The original code assumes that GetRawVideoFrame?() will return a frame >= to the frame number requested - this is no longer true.

D2_v2.patch fixes this and is significantly faster - previously it was seeking to every frame, even when sequential - now it only seeks when it needs to jump to a specified frame.

While this patch will allow the code to work, I will note, as other developers have commented, that the detection code is both slow, and no better than the Classic Commercial detector code - my experience confirms those anecdotal comments.

comment:5 Changed 5 years ago by paulh

  • Milestone changed from 0.26.1 to 0.27

There is a patch so hopefully someone will look this for 0.27?

comment:6 Changed 5 years ago by stichnot

The issue appears to be an off-by-one mismatch in frame numbering, in the case of seeking to a specific frame number versus querying the number of the most recently rendered frame. In the original code, this results in continually asking to seek to the same frame. I suspect that the 'Remove the "Seek to exact frame" setting' commit made the logic more rigorous and exposed a preexisting issue in the experimental commflagging code.

Apart from that, it's a nice find that we can greatly speed up next-frame seeking by avoiding actual seeking.

comment:7 Changed 5 years ago by Jim Stichnoth <jstichnoth@…>

  • Resolution set to fixed
  • Status changed from accepted to closed

In 03a766768cb6f9d050e20e8d050fb6428e9ff909/mythtv:

Fixes #11141, a frame numbering mismatch in the commflagger.

The mismatch is between the frame numbering for seeking and the frame
numbering for the video display. Also, optimize the "seek to next
frame" case.

This only affects the "experimental" commflagger.

Add Comment

Modify Ticket

as closed The owner will remain stichnot.
The resolution will be deleted. Next status will be 'new'.

E-mail address and user name can be saved in the Preferences.

Note: See TracTickets for help on using tickets.