[mythtv] Ticket #11899: Garbled CCs on some stations

[Jim Stichnoth]

On Mon, Oct 14, 2013 at 10:05 PM, faginbagin <mythtv at hbuus.com> wrote:

> On 10/12/2013 5:19 PM, MythTV wrote:
> > #11899: Garbled CCs on some stations
> > -----------------------------------+----------------------------
> >   Reporter:  faginbagin <mythtv@…>  |          Owner:  stichnot
> >       Type:  Bug Report - General   |         Status:  accepted
> >   Priority:  minor                  |      Milestone:  unknown
> > Component:  MythTV - Captions      |        Version:  0.27-fixes
> >   Severity:  medium                 |     Resolution:
> >   Keywords:                         |  Ticket locked:  0
> > -----------------------------------+----------------------------
> >
> > Comment (by stichnot):
> >
> >   A few observations so far.
> >
> >   1. Mostly (but not entirely), the garbling is because roughly half the
> >   characters aren't being displayed.
> >
> >   2. xine shows almost exactly the same garbled captions as MythTV.
> >
> >   3. As noted, ccextractor (version 0.65 in my case) produces very clean
> >   captions.
> >
> >   4. If you combine the captions from fields 0 and 1 by setting field=0
> at
> >   the beginning of CC608Decoder::FormatCCField(), most (but not all) of
> the
> >   caption characters are correctly displayed.
> >
> >   Especially in light of the xine behavior, I suspect an upstream ffmpeg
> >   problem, though it's also possible that it's a problem in our
> >   avformatdecoder.cpp.
>
> Thanks for looking at this. Some questions:
>
> Where is the "upstream" version of ffmpeg used by xine and mythtv? When I
> look at:
> https://github.com/FFmpeg/FFmpeg/blob/release/1.2/libavcodec/mpeg12.c
>
> http://git.videolan.org/?p=ffmpeg.git;a=blob_plain;f=libavcodec/mpeg12.c;hb=refs/heads/release/1.2
> these versions of mpeg12.c don't have any reference to the data members
> tmp_atsc_cc_len or tmp_scte_cc_len, which leads me to believe I'm not
> looking at the right upstream repositories, at least when it comes to CC
> support in ffmpeg. I found this commit:
>
> http://git.videolan.org/?p=ffmpeg.git;a=commit;h=33d699a4e73d5281b2cfcd0fa355c0d80241dd23
> which seems to match the one referenced in this mythtv commit:
>
> https://github.com/MythTV/mythtv/commit/9c728cb8f19100e9976196709b8258480e72d30b
> This leads me to believe videolan is the source of mythtv's ffmpeg code,
> but videolan must do something different when it comes to CC processing.
>

I didn't think to actually check the pristine upstream version of ffmpeg.
 It looks like most of the code in mpeg_decode_user_data() is a
MythTV-specific addition in our repository.  This explains a lot.


>
> I've skimmed at the mpeg12.c code and wondered about the way it copies CC
> data from the source video stream into temp buffers that are later copied
> to other buffers by mpegvideo.c and then handled by avformatdecoder.cpp. I
> suspect the mpeg12.c code may be stripping out needed info from the control
> bytes. I haven't looked "super" close, but maybe its time to do so. It
> would help to know where that code came from.
>

I believe the error is introduced in the parsing stage, not the copying
stages.  Specifically, at
https://github.com/MythTV/mythtv/blob/master/mythtv/external/FFmpeg/libavcodec/mpeg12.c#L2258
with the rollup.mpg sample, I can set a breakpoint and observe it
parsing cc_data_1 = 'X' and cc_data_2 = 'E', and it subsequenty adds this
bogus pair of characters to the stream.  I've compared this code, the
corresponding ccextractor code (user_data() in es_userdata.cpp in the
section with the comment "// SCTE 20 user data"), and section 5.5 of the
spec http://www.scte.org/documents/pdf/standards/SCTE%2020%202012.pdf, line
by line, and didn't find any meaningful differences that would explain this
"XE" difference.

There are a couple of differences from the ccextractor code that explain
most of the cc608 omissions, at least in the rollup.mpg sample:

==========
diff --git a/mythtv/external/FFmpeg/libavcodec/mpeg12.c
b/mythtv/external/FFmpeg/libavcodec/mpeg12.c
index 6d24889..a813600 100644
--- a/mythtv/external/FFmpeg/libavcodec/mpeg12.c
+++ b/mythtv/external/FFmpeg/libavcodec/mpeg12.c
@@ -2257,13 +2257,13 @@ static void mpeg_decode_user_data(AVCodecContext
*avctx,
                 uint8_t line_offset = get_bits(&gb, 5);
                 uint8_t cc_data_1 = av_reverse[get_bits(&gb, 8)];
                 uint8_t cc_data_2 = av_reverse[get_bits(&gb, 8)];
-                uint8_t type = (1 == field_no) ? 0x00 : 0x01;
+                uint8_t type = (2 == field_no) ? 0x01 : 0x00;
                 (void) priority; // we use all the data, don't need
priority
                 marker &= get_bits(&gb, 1);
                 // dump if marker bit missing
                 valid = marker;
                 // ignore forbidden and repeated (3:2 pulldown) field
numbers
-                valid = valid && (1 == field_no || 2 == field_no);
+                valid = valid && (0 != field_no);
                 // ignore content not in line 21
                 valid = valid && (11 == line_offset);
                 if (!valid)
diff --git a/mythtv/libs/libmythtv/cc608decoder.cpp
b/mythtv/libs/libmythtv/cc608decoder.cpp
index e055718..e25257e 100644
--- a/mythtv/libs/libmythtv/cc608decoder.cpp
+++ b/mythtv/libs/libmythtv/cc608decoder.cpp
@@ -143,6 +143,7 @@ static const QChar extendedchar3[] =

 void CC608Decoder::FormatCCField(int tc, int field, int data)
 {
+    field = 0;
     int b1, b2, len, x;
     int mode;

==========
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mythtv.org/pipermail/mythtv-dev/attachments/20131015/cf9de7cf/attachment.html>