Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#6889 closed enhancement (fixed)

New commercial detection method to flag Pre and post roll only

Reported by: Paul <Paul@…> Owned by: cpinkham
Priority: minor Milestone: 0.22
Component: MythTV - General Version: head
Severity: low Keywords: mythcommflag
Cc: Ticket locked: no

Description

This commercial flagger is designed for commercial free channels. It simply flags the Preroll and post roll periods as commercials. The implementation class uses the classic Commercial detector to find the blank frame or scene change that is closest to the location where the show should have started or stopped. In general it works better on the preroll than the postroll.

Attachments (2)

PrePostRollFlagger.patch (22.8 KB) - added by Paul <Paul@…> 10 years ago.
Patch containing implementation of PrePostRollFlagger?
PrePostRollFlagger.1.patch (24.6 KB) - added by Paul <Paul@…> 10 years ago.
Full patch. Superseeds previous version.

Download all attachments as: .zip

Change History (7)

Changed 10 years ago by Paul <Paul@…>

Attachment: PrePostRollFlagger.patch added

Patch containing implementation of PrePostRollFlagger?

comment:1 Changed 10 years ago by cpinkham

Owner: changed from Isaac Richards to cpinkham
Status: newassigned

comment:2 Changed 10 years ago by cpinkham

I don't see where you are waiting until after the pre-roll to being flagging. In ::go(), requiredHeadStart is set to requiredBuffer which is set to 120, you never add in preRoll.

Can you wrap your long lines at 80 columns by splitting strings and putting function or QString args on the next line, etc.?

Myth's coding standard says that opening brackets for blocks of code go on the next line, not on the if/while/for/function line, please correct these as well.

Duplicated code here: (COMM_FRAME_SCENE_CHANGE | COMM_FRAME_BLANK | COMM_FRAME_SCENE_CHANGE)

Please include a patch to libs/libmyth/programinfo.cpp to add this detection method to the SkipTypeToString?() and GetPreferredSkipTypeCombinations?() functions there so that the user would not have to edit the database to change the default or channel-specific flagging methods.

How accurate is this and what kind of pre/post roll did you use when testing. It appears that you are only looking for the closest blank frame or scene change frame to the pre/post roll times.

If you can correct these issues in an updated patch, I should be able to get this in before the feature freeze on Monday.

Changed 10 years ago by Paul <Paul@…>

Attachment: PrePostRollFlagger.1.patch added

Full patch. Superseeds previous version.

comment:3 in reply to:  2 Changed 10 years ago by Paul <Paul@…>

Replying to cpinkham:

I don't see where you are waiting until after the pre-roll to being flagging. In ::go(), requiredHeadStart is set to requiredBuffer which is set to 120, you never add in preRoll.

I think it is taking the preroll into account by using the using the startedAt time instead of the recordingStartedAt time.

Can you wrap your long lines at 80 columns by splitting strings and putting function or QString args on the next line, etc.?

Myth's coding standard says that opening brackets for blocks of code go on the next line, not on the if/while/for/function line, please correct these as well.

Duplicated code here: (COMM_FRAME_SCENE_CHANGE | COMM_FRAME_BLANK | COMM_FRAME_SCENE_CHANGE)

Please include a patch to libs/libmyth/programinfo.cpp to add this detection method to the SkipTypeToString?() and GetPreferredSkipTypeCombinations?() functions there so that the user would not have to edit the database to change the default or channel-specific flagging methods.

These should all be addressed by the new patch.

How accurate is this and what kind of pre/post roll did you use when testing. It appears that you are only looking for the closest blank frame or scene change frame to the pre/post roll times.

If you can correct these issues in an updated patch, I should be able to get this in before the feature freeze on Monday.

On my system i generally use a 5 minute preroll and a 1 minute post roll. In my experience the pre-roll accuracy is pretty good. However, the postroll accuracy leaves a lot to be desired. My theory at this point is that shows generally end before the scheduled times to allow time for commercials or previews from the station. You are correct that i am simply looking for whichever blank frame/scene change is closest to the estimated preroll/postroll frames. I haven't put much effort into improving it at this point as the preroll was the most important for me, and i just coded it shortly before the feature freeze was announced.

comment:4 Changed 10 years ago by cpinkham

Resolution: fixed
Status: assignedclosed

(In [21446]) Add a new 'commercial' flagger "Pre & Post Roll" that is

DESIGNED TO FLAG -=-=-=ONLY=-=-=- THE PRE/POST ROLL PARTS

of a recording so that they can be easily skipped. This is meant to be used on channels without commercials such as movie channels that have their own 'commercials' prior to and following a movie. This flagger will attempt to highlight those areas so that they can be easily skipped during playback.

Closes #6889 using patch by Paul ?Turpin?.

comment:5 Changed 8 years ago by Github

Correct copy/paste error in postroll commercial detection

Corrects an incorrect variable use that was missed when copying a similar block of code added in 9f5ee51db82.

Fixes #10120 Refs #6889

Signed-off-by: Raymond Wagner <rwagner@…>

Branch: master Changeset: 39c61832fb8e9e17fecf478426e67a223f6bd008

Note: See TracTickets for help on using tickets.