Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 19 months ago

Closed 8 months 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

Description

The current code just keeps requesting frame 0.

Attachments (2)

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

Download all attachments as: .zip

Change History (9)

Changed 19 months ago by Wayne McDougall <waynemcdougall@…>

comment:1 Changed 19 months 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 19 months ago by stuartm

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

comment:3 Changed 19 months 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 19 months ago by Wayne McDougall <waynemcdougall@…>

comment:4 Changed 19 months 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 9 months 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 8 months 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 8 months 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

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


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

 
Note: See TracTickets for help on using tickets.