Opened 11 years ago

Closed 11 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: Jim Stichnoth
Priority: minor Milestone: 0.27
Component: MythTV - Mythcommflag Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

The current code just keeps requesting frame 0.

Attachments (2)

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

Download all attachments as: .zip

Change History (9)

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

Attachment: D2.patch added

comment:1 Changed 11 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.: https://github.com/MythTV/mythtv/commit/4d0bbbe1e54aaa91abbd233b7e5a1a48103462f2

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 11 years ago by stuartm

Owner: changed from cpinkham to Jim Stichnoth
Status: newassigned

comment:3 Changed 11 years ago by Jim Stichnoth

Milestone: unknown0.26.1
Status: assignedaccepted
Version: UnspecifiedMaster Head

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

Attachment: D2_v2.patch added

comment:4 Changed 11 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 11 years ago by paulh

Milestone: 0.26.10.27

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

comment:6 Changed 11 years ago by Jim Stichnoth

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 11 years ago by Jim Stichnoth <jstichnoth@…>

Resolution: fixed
Status: acceptedclosed

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.

Note: See TracTickets for help on using tickets.