Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#12308 closed Patch - Bug Fix (Fixed)

Improve mythcommflag accuracy

Reported by: faginbagin <mythtv@…> Owned by: cpinkham
Priority: minor Milestone: unknown
Component: MythTV - Mythcommflag Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

The focus of the patches submitted with this ticket was on improving accuracy for SD recordings, but care has also been given to make sure commflagging for HD recordings was either unchanged or improved.

In summary, I found and fixed the following problems:

1) Mythcommflag assumed that the number of bytes per scan line for the luma array equals the video width. That may have been true before mythcommflag was changed to use ffmpeg's lowres decoding option, but it is not true when the lowres option is used. The bytes per scan line are actually the width rounded up to a multiple of 16. Some of the SD resolutions, when divided by 4 (as is the case for the lowres option), are not multiples of 16. The patch, mcf-bytesPerLine.patch, fixes this problem, and improved accuracy from 51% to 54%. More on how I came up with these percentages later.

2) Many years before the lowres option was added to improve performance, a default value for the CommDetectBorder? setting was set to 20. It is used to exclude CommDetectBorder? pixels from the top, bottom, left and right of the frame from examination. It should, for example, exclude any VBI data that might be visible at the top of a frame or parts of the video that would be hidden due to overscan. The default value was never adjusted to account for lowres decoding, nor did it take into consideration the variety of video widths and heights that exist today. The patch, mcf-border.patch, assumes the CommDetectBorder? setting was established for 720 height video and uses those two numbers and the lowres video height to compute the border used. For example: If video source height = 480 then border = 20 * 480 / 4 / 720 = 2 If video source height = 720 then border = 20 * 720 / 4 / 720 = 5 If video source height = 1080 then border = 20 * 1080 / 4 / 720 = 7 mcf-border.patch improves accuracy to 67%.

3) During the "Initial Block pass" where scoring based on different criteria is applied, there was one test that didn't take into consideration whether logo detection had failed. The patch, mcf-nologo.patch, addresses that oversight, and improves accuracy to 69%.

4) The CommDetectBorder? setting did dual duty, as it was also used to limit the area examined by logo detection. The mcf-border.patch for problem 2) did not help a number of SD recordings, because it allowed logo detection to examine letter-boxed areas of SD video and caused logo detection to fail on recordings where it used to work. To detect logos, we need to exclude letter-boxed areas from SD video, but if we exclude too much from HD video, we'll miss the logo. Using the same border of 16 for both SD & HD with no scaling seems to be a good compromise. The patch, mcf-logo-border.patch, adds a new setting, CommDetectLogoBorder? with a default value of 16 and uses it as the border for logo detection. It improves accuracy to 72%.

5) Logo detection required a minimum number of pixels in a mask found by examining several video frames to be greater than 50 to be considered a valid logo mask. No consideration of the variety of video resolutions was taken into consideration. And the 50 pixel minimum was not adjusted when lowres decoding was implemented. The patch, mcf-logo-pixInMask.patch, assumes the 50 pixel value was established for 1280x720 video and uses that area to compute a minimum pixel value using this formula: 50 * (width*height) / (1280*720 / 16). It improves accuracy to 75%.

6) Mythcommflag assumed that video can be either normal, letter-boxed or pillar-boxed, but not both letter-boxed and pillar-boxed. I have found there are often instances where it can be both letter-boxed and pillar-boxed, especially in commercials found in SD recordings and on the SD version of our PBS channel. The patch, mcf-letter-pillar.patch changes the enum frameFormat into a bit mask allowing the detection of video that's both letter and pillar-boxed. This patch did not make any overall change to accuracy, at least not with the set of recordings I chose to measure results. But it didn't make things worse, and I do have other recordings where it helps. I just wasn't prepared to go back and repeat all the tests with a different set of recordings.

The details:

To gain more insight into how mythcommflag works, I found debugging code that Chris Pinkham had posted on mythtv-users but had never put under source control, even though there was ifdef'ed code referencing the debugging code. Here's his post: https://www.mythtv.org/pipermail/mythtv-dev/2006-January/043094.html The debugging code can be used to visualize logo detection and also to see video frames as they are processed. I'm glad I found it, because it is what lead to the bytes per scan line discovery. I could clearly see that some SD recordings were severely skewed. The patch, mcf-display-debug.patch, adds the debugging code. It is not enabled by default, but can be, by executing these commands in the mythtv/programs/mythcommflag directory: $ make clean $ qmake DEFINES+=SHOW_DEBUG_WIN $ make The resulting version of mythcommflag will prompt you to hit enter during each pass of logo detection. You will also need to hit enter as each video frame is decoded, although there is no prompt. To disable the debugging code, execute the following: $ make clean $ qmake $ make

So how did I come up with the accuracy percentages mentioned in my summary? I wanted to figure out how to measure the impact of any code changes. So, I picked a set of 20 test recordings, one from each channel I regularly record. 13 are from SD channels, 7 are from HD channels.

The SD recordings have a mix of resolutions, all have a height of 480, but the widths vary: 2 are 528, 1 is 640, 5 are 704 (two of those have some commercials that are 720), and 5 are 720. 11 have an aspect ratio of 4:3, 2 have 16:9 aspect ratio. Of the 4:3 recordings: 5 are letter-boxed and 6 are full-screen. Of the 16:9 recordings, one is wide-screen and one is pillar-boxed.

The HD recordings come in the two ATSC HD resolutions: 3 are 1280x720p, 4 are 1920x1080i.

I then used the cut list editor to mark commercial breaks. This gave me something to compare to the commercial marks found by different iterations of mythcommflag. If a starting commercial mark was within 200 frames of a starting cut mark, I counted it as a match. And if an ending commercial mark was within 200 frames of an ending cut mark, I counted it as a match. These were divided by the total number of cut marks (both start and end) to arrive at a measure of accuracy. I used some shell scripting, a little java program to do the calculations given the recordedmarkup entries for a given recording, and a spreadsheet to get the numbers.

In all tests, I only used the default commercial detection method: "All Available Methods" (CommercialSkipMethod? = 7). Before making any changes, I collected statistics with "Strict commercial detection" disabled and enabled (AggressiveCommDetect? = 0 and 1). Turns out that "Strict commercial detection" is more accurate. For the unpatched mythcommflag the difference is 36% vs 51%. For the patched mythcommflag, the difference is 66% vs 75%.

I'm also attaching mcf-combined.patch, which combines all the patches described above into one patch.

Attachments (8)

mcf-display-debug.patch (7.8 KB) - added by faginbagin <mythtv@…> 5 years ago.
mcf-bytesPerLine.patch (12.5 KB) - added by faginbagin <mythtv@…> 5 years ago.
mcf-border.patch (1.5 KB) - added by faginbagin <mythtv@…> 5 years ago.
mcf-nologo.patch (654 bytes) - added by faginbagin <mythtv@…> 5 years ago.
mcf-logo-border.patch (2.1 KB) - added by faginbagin <mythtv@…> 5 years ago.
mcf-logo-pixInMask.patch (1.9 KB) - added by faginbagin <mythtv@…> 5 years ago.
mcf-letter-pillar.patch (2.9 KB) - added by faginbagin <mythtv@…> 5 years ago.
mcf-combined.patch (26.9 KB) - added by faginbagin <mythtv@…> 5 years ago.

Download all attachments as: .zip

Change History (32)

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-display-debug.patch added

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-bytesPerLine.patch added

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-border.patch added

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-nologo.patch added

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-logo-border.patch added

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-logo-pixInMask.patch added

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-letter-pillar.patch added

Changed 5 years ago by faginbagin <mythtv@…>

Attachment: mcf-combined.patch added

comment:1 Changed 5 years ago by warpme@…

Hi, I was trying to play with those patches - but unfortunately I'm receiving following error building on current master: (In fact I want to play with those patches on 0.27-fixes - but error is similar)

g++ -c -m64 -pipe -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DPIC -fomit-frame-pointer -fPIC -msse -pthread -g -Wall -Wpointer-arit
h -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -fvisibility-inlines-hidden -Wmissing-declarations -Wno-switch -Woverloaded-virtual
 -funit-at-a-time -D_REENTRANT -DMMX -D_GNU_SOURCE -DQT_NO_DEBUG -DQT_SQL_LIB -DQT_XML_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_CORE_L
IB -DQT_SHARED -I/usr/share/qt/mkspecs/linux-g++-64 -I. -I/usr/include/QtCore -I/usr/include/QtNetwork -I/usr/include/QtGui -I/usr/i
nclude/QtXml -I/usr/include/QtSql -I/usr/include -I/usr/include -I/usr -I/usr/include/libxml2 -I../../external/qjson/include -I../..
 -I../../libs -I../../libs/libmyth -I../../libs/libmyth/audio -I../../libs/libmythtv -I../../external/FFmpeg -I../../libs/libmythupn
p -I../../libs/libmythui -I../../libs/libmythmetadata -I../../libs/libmythlivemedia -I../../libs/libmythbase -I../../external/libmyt
hdvdnav/dvdnav -I../../external/libmythdvdnav/dvdread -I../../external/libmythbluray -I../../external/libmythsoundtouch -I../../exte
rnal/libsamplerate -I../../libs/libmythtv/mpeg -I../../libs/libmythtv/vbitext -I../../libs/libmythservicecontracts -I../../libs/libm
ythprotoserver -I/usr/X11R6/include -I. -o Histogram.o Histogram.cpp
Histogram.cpp: In member function ‘void Histogram::generateFromImage(VideoFrame*, unsigned int, unsigned int, unsigned int, unsigned
 int, unsigned int, unsigned int, unsigned int, unsigned int)’:
Histogram.cpp:34:36: error: invalid use of incomplete type ‘VideoFrame {aka struct VideoFrame_}’
In file included from Histogram.cpp:1:0:
Histogram.h:4:16: error: forward declaration of ‘VideoFrame {aka struct VideoFrame_}’
Histogram.cpp:35:29: error: invalid use of incomplete type ‘VideoFrame {aka struct VideoFrame_}’
In file included from Histogram.cpp:1:0:
Histogram.h:4:16: error: forward declaration of ‘VideoFrame {aka struct VideoFrame_}’
make[2]: *** [Histogram.o] Error 1
make[2]: Leaving directory `/home/piotro/backend-dev/ABS/mythtv-master/src/mythtv-master-build/mythtv/programs/mythcommflag'
make[1]: *** [sub-mythcommflag-make_default] Error 2
make[1]: *** Waiting for unfinished jobs....

comment:2 Changed 5 years ago by George Nassas <gnassas@…>

First off, thanks for this effort!

I'm also seeing the build errors, the fixes are pretty simple and both in mythtv/mythtv/programs/mythcommflag:

in Histogram.cpp

#include "frame.h"

should be:

#include "mythframe.h" #include "libavutil/frame.h"

and in ClassicLogoDetector?.cpp

#include "frame.h"

should be:

#include "libavutil/frame.h"

No biggie.

comment:3 in reply to:  1 Changed 5 years ago by faginbagin <mythtv@…>

Replying to warpme@…:

Hi, I was trying to play with those patches - but unfortunately I'm receiving following error building on current master: (In fact I want to play with those patches on 0.27-fixes - but error is similar)

g++ -c -m64 -pipe -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DPIC -fomit-frame-pointer -fPIC -msse -pthread -g -Wall -Wpointer-arit
h -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -fvisibility-inlines-hidden -Wmissing-declarations -Wno-switch -Woverloaded-virtual
 -funit-at-a-time -D_REENTRANT -DMMX -D_GNU_SOURCE -DQT_NO_DEBUG -DQT_SQL_LIB -DQT_XML_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_CORE_L
IB -DQT_SHARED -I/usr/share/qt/mkspecs/linux-g++-64 -I. -I/usr/include/QtCore -I/usr/include/QtNetwork -I/usr/include/QtGui -I/usr/i
nclude/QtXml -I/usr/include/QtSql -I/usr/include -I/usr/include -I/usr -I/usr/include/libxml2 -I../../external/qjson/include -I../..
 -I../../libs -I../../libs/libmyth -I../../libs/libmyth/audio -I../../libs/libmythtv -I../../external/FFmpeg -I../../libs/libmythupn
p -I../../libs/libmythui -I../../libs/libmythmetadata -I../../libs/libmythlivemedia -I../../libs/libmythbase -I../../external/libmyt
hdvdnav/dvdnav -I../../external/libmythdvdnav/dvdread -I../../external/libmythbluray -I../../external/libmythsoundtouch -I../../exte
rnal/libsamplerate -I../../libs/libmythtv/mpeg -I../../libs/libmythtv/vbitext -I../../libs/libmythservicecontracts -I../../libs/libm
ythprotoserver -I/usr/X11R6/include -I. -o Histogram.o Histogram.cpp
Histogram.cpp: In member function ‘void Histogram::generateFromImage(VideoFrame*, unsigned int, unsigned int, unsigned int, unsigned
 int, unsigned int, unsigned int, unsigned int, unsigned int)’:
Histogram.cpp:34:36: error: invalid use of incomplete type ‘VideoFrame {aka struct VideoFrame_}’
In file included from Histogram.cpp:1:0:
Histogram.h:4:16: error: forward declaration of ‘VideoFrame {aka struct VideoFrame_}’
Histogram.cpp:35:29: error: invalid use of incomplete type ‘VideoFrame {aka struct VideoFrame_}’
In file included from Histogram.cpp:1:0:
Histogram.h:4:16: error: forward declaration of ‘VideoFrame {aka struct VideoFrame_}’
make[2]: *** [Histogram.o] Error 1
make[2]: Leaving directory `/home/piotro/backend-dev/ABS/mythtv-master/src/mythtv-master-build/mythtv/programs/mythcommflag'
make[1]: *** [sub-mythcommflag-make_default] Error 2
make[1]: *** Waiting for unfinished jobs....

I'm confused, did you get the same errors trying to compile against fixes/0.27? Or maybe you first tried compiling against master? There was a change in master where libmythv/frame.h was renamed to libmythtv/mythframe.h, but that hasn't been applied to fixes/0.27. My patches should compile without error on 0.27. On master, you probably need to make the changes George outlines.

comment:4 Changed 5 years ago by warpme@…

Oh forgive me about remark regarding 0.27-fixes - I was using wrong GIT tree :-( I applied patch to current master. So far on some channels I see improvement. After some more observations I'll report how it goes in my environment. Anyway - big thx for You work!

comment:5 Changed 5 years ago by jpoet

I just tested these patches against my most problematic channel, and they do make a noticeable improvement. Detection is still not 100% perfect, but is much better.

Test 1:
Without the patch
     Actual transition points: 12
     Accurately detected:       6
     Missed:                    6
     Erroneous:                 1
With the patches
     Actual transition points: 12
     Accurately detected:       9  (1 was slightly off, but close enough)
     Missed:                    3
     Erroneous:                 1

Test 2:
Without the patch
     Actual transition points: 12
     Accurately detected:       8
     Missed:                    4
     Erroneous:                 1
With the patches
     Actual transition points: 12
     Accurately detected:      11
     Missed:                    1
     Erroneous:                 1

Test 3:
Without the patch
     Actual transition points: 12
     Accurately detected:       5
     Missed:                    7
     Erroneous:                 3
With the patches
     Actual transition points: 12
     Accurately detected:       9
     Missed:                    3
     Erroneous:                 1

Chris, if you don't have time to review these patches I can take a stab at it.

comment:6 in reply to:  5 ; Changed 5 years ago by faginbagin <mythtv@…>

Replying to jpoet:

I just tested these patches against my most problematic channel, and they do make a noticeable improvement. Detection is still not 100% perfect, but is much better.

<snip>

Many thanks for checking out my work. Looks like your results are similar to mine, an increase in accuracy from 51% to 75% for "Strict commercial detection".

I'm curious, what procedure/tools did you use to generate the data?

comment:7 in reply to:  6 Changed 5 years ago by jpoet

Replying to faginbagin <mythtv@…>:

I'm curious, what procedure/tools did you use to generate the data?

In mythfrontend, start a show playing and then hit 'E' to edit the cutlist. Hit [End] to load the flagged commercials into the cutlist. Use the up/down arrows to adjust the increment, and the left/right arrows to scrub through the video. Scrub the video noting where the actual transitions are, comparing them to what was detected.

comment:8 Changed 5 years ago by bandvhodges@…

I am applying your patch to a fresh svn. I cannot find the include that you mention: #include "mythframe.h" #include "libavutil/frame.h"

Can you tell me how to get them?

Thank you, Bill HOdges

comment:9 Changed 5 years ago by faginbagin <mythtv@…>

Hi Bill,

I assume you mean the master branch when you refer to a "fresh svn" and not the fixes/0.27 branch. If so, according to George, you need to edit Histogram.cpp, changing line 6 from:

#include "frame.h"

to

#include "mythframe.h"
#include "libavutil/frame.h"

And you need to change line 10 in ClassicLogoDetector?.cpp from:

#include "frame.h"

to

#include "libavutil/frame.h"

comment:10 Changed 4 years ago by cdhunter2@…

Any chance of this getting committed? I'd love to not have to rebuild mythcommflag every time I update 0.27-fixes.

comment:11 Changed 4 years ago by jpoet

In my testing, these patches improve the detection for most channels. However, on some channels I have shows which went from good detection (9 of 12) to terrible (3 of 12). I would feel much better about committing these patches if they were at least no-worse than the detection without them.

It may be impossible to achieve an automatic set of parameters for all channels. The solution may be to make some of these changes user-tunable, with profiles which could be saved, and then selected on a channel by channel basis, or a recording by recording basis. My knowledge of the commflag code is not in-depth enough to know what parameters would need to be tunable, however.

faginbagin mentioned to me that setting the percentage of matching frames during the logo detection could be a prime target for a tunable. There are likely other parameters which would need to be tweaked differently from some channels (shows) compared to others.

I will play with the logo detection parameters and see if I can 'fix' the detection of the show which dropped from a 9 of 12 accuracy to a 3 of 12.

In the US (at least), adding detecting for the "TV Parental Guidelines" logo would greatly help in the detection coming out-of-commercial. From the Wikipedia article:

For the first 15 seconds of every rated program lasting a half-hour or less, a large rating icon appears in the upper-left hand corner of the screen. The icon was much smaller until June 2005 and only appeared on-screen for 7.5 seconds. For every rated program running an hour or longer, a rating appears in the upper-left hand corner of the television screen at the beginning of each half hour. Starting in June 2005, many networks now display the ratings after every commercial break, in addition to the beginning of the program. If a network uses a 16:9 format for its presentation, generally all rating icons appear in the 4:3 safe area of the feed.

The commflag code does not currently find or use that logo. Personally, I don't mind so much when the detector misses the change from show to commercial, but I would like it to nail the transition from commercial to shoe. Using the "TV Parental Guidelines" logo should make that possible, I would think, for most shows -- at least in the US.

comment:12 Changed 4 years ago by cdhunter2@…

Your show that went from 9 of 12 to 3 of 12 was it HD? Were you able to tell which "patch" caused the drop? My problems are always with SD content. I would love to have the changes that only affect SD to be committed.

comment:13 Changed 4 years ago by jpoet

The show in question is "Ultimate Factories" on National Geographic HD. The issue seems to be that NatGeo? is advertising other NatGeo? shows in the upper right corner, while showing their logo in the lower right corner. mythcommflag thinks that the Ad in the upper right corner is part of the logo, but that add changes every several minutes, resulting in the logo not matching...

The unpatched mythcommflag code also fails to properly detect that the logo in just in the lower right corner, but still manages to find the commercials without logo detection. With the patched commflagger, miss-detecting the logo results in almost total failure.

As a hack, I changed the code to only look for the logo in the lower right corner, which resulted in the patched code detecting the same commercials as the unpatched code.

I don't know why the unpatched code is so much better sans logo, than the patched code. When the logo is properly detected, the patched code does seem to be superior. More testing is necessary...

comment:14 Changed 4 years ago by faginbagin <mythtv@…>

I suspect the problem with my patches degrading commercial detection in HD content can be traced to one patch, "mcf-nologo.patch". That's based on the "Hell on Wheels" sample that jpoet provided. But I haven't found the time to confirm my suspicions. It's a small one line patch that seemed to make a small improvement with my original sample of recordings but may prove to make things worse when more recordings are examined. If anyone is willing, please revert that one line change and let us know your results.

While I agree with jpoet that tuning comm flagging based on criteria such as channel or series could increase accuracy, I think that would be quite a bit of work to implement and I doubt most users would be willing to take the time to configure such a feature. So I hope that reverting that one line patch proves to be a simpler solution that is no worse than before for HD and much better for SD.

comment:15 Changed 4 years ago by jpoet

I removed mcf-nologo.patch and re-ran it against "Ultimate Factories - The Assembly Line", and it did not help.

comment:16 Changed 4 years ago by cdhunter2@…

I'm not much help since I don't seem to have any HD content that gets worse with the changes but would it be too much to ask to figure out which patch causes the issue? I assumed most of them were SD specific. Maybe I was wrong.

comment:17 Changed 4 years ago by jpoet

Apparently I blew the earlier test with mcf-nologo.patch removed. I went through and tested each patch, and it is indeed mcf-nologo.patch which causes the regression when the logo is not detected.

comment:18 Changed 4 years ago by faginbagin <mythtv@…>

Cool! I was bummed when you first reported removing that patch did not help. So there's hope that the other patches might be accepted, right?

comment:19 Changed 4 years ago by cdhunter2@…

Awesome! Is there anything else that needs to be done to get this committed besides removing the one patch? I can help with any more testing if needed. So far I'm really happy with the improvements to SD recordings and never noticed any issues with HD.

comment:20 Changed 4 years ago by jpoet

Resolution: Fixed
Status: newclosed

comment:21 Changed 4 years ago by jpoet

Version: 0.27.4Master Head

comment:22 Changed 4 years ago by faginbagin <mythtv@…>

Thank You! Any chance you'll be backporting to fixes/0.27?

comment:23 Changed 4 years ago by jpoet

I will backport it in a few weeks, assuming no one finds any issues from it while using master.

comment:24 Changed 4 years ago by cdhunter2@…

Has there been any issues found with these changes in master? I'm only running fixes/0.27 on my systems so I haven't been able to test.

Note: See TracTickets for help on using tickets.