[FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Scott Theisen
> Sent: Thursday, February 3, 2022 11:17 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect
> yellow appearance of dvd subtitles
> 
> On 2/3/22 17:10, Soft Works wrote:
> >
> >> -----Original Message-----
> >> From: ffmpegagent <ffmpegagent at gmail.com>
> >> Sent: Tuesday, January 4, 2022 3:19 AM
> >> To: ffmpeg-devel at ffmpeg.org
> >> Cc: softworkz <softworkz at hotmail.com>; softworkz
> >> <softworkz at hotmail.com>
> >> Subject: [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance
> of
> >> dvd subtitles
> >>
> >> From: softworkz <softworkz at hotmail.com>
> >>
> >> The guess_palette() implementation is questionable in itself
> >> as its results don't match those from other DVD subtitle decoders.
> >>
> >> This commit starts cleanup by fixing an obvious bug which has made
> >> certain DVD subs appear yellow instead of white or grey for more
> than
> >> 10 years..
> >>
> >> Signed-off-by: softworkz <softworkz at hotmail.com>
> >> ---
> >>      avcodec/dvdsubdec: fix incorrect yellow appearance of dvd
> >> subtitles
> >>
> >>      Fixes an age-old bug in decoding DVD subtitles.
> >>
> >>      Ever wondered why certain DVD subtitles are shown in yellow
> color
> >> when
> >>      ffmpeg is involved...
> >>
> >> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> >> ffstaging-16%2Fsoftworkz%2Fpatch_dvdsubdec_fix-v1
> >> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> >> ffstaging-16/softworkz/patch_dvdsubdec_fix-v1
> >> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/16
> >>
> >>   libavcodec/dvdsubdec.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> >> index 52259f0730..a3fdb535a5 100644
> >> --- a/libavcodec/dvdsubdec.c
> >> +++ b/libavcodec/dvdsubdec.c
> >> @@ -400,7 +400,7 @@ static int decode_dvd_subtitles(DVDSubContext
> >> *ctx, AVSubtitle *sub_header,
> >>                   } else {
> >>                       sub_header->rects[0]->nb_colors = 4;
> >>                       guess_palette(ctx, (uint32_t*)sub_header-
> >>> rects[0]->data[1],
> >> -                                  0xffff00);
> >> +                                  0xffffff);
> >>                   }
> >>                   sub_header->rects[0]->x = x1;
> >>                   sub_header->rects[0]->y = y1;
> >>
> >> base-commit: 573b6b8a607398c5f34108efda9c29d41c5727ff
> >> --
> >> ffmpeg-codebot
> > Ping. (no maintainer seems to be registered for this)
> 
> MythTV has used this fix since 2010-06-04.  See
> https://github.com/ulmus-
> scott/FFmpeg/commit/e2b1a6ee63c0cccc1ac9c82d24e2e6cfffeb2bfc
> 
> libavcodec/dvdsubdec.c: default to white instead of yellow
> 
> from Improved display of DVD subtitles in containers other than the
> original.
> https://github.com/MythTV/mythtv/commit/2510a0821ea4453eb9b34dd96e68ff
> 0441459d0b
> references: https://code.mythtv.org/trac/ticket/8222

Thanks for the references. I have compared the ffmpeg output
with a range of (non-ffmpeg-based) players including VLC, 
an example where I had a screen photo from a hardware player
and two subtitle editing tools. 
None of those showed them yellow.


> For whatever strange reason, ffmpeg has always used yellow:
> https://github.com/FFmpeg/FFmpeg/commit/240c1657dcd45adc0e63ef947b9209
> 19071ec1f7

My suspicion is that 0xffff00 was written assuming RGBA order instead
of ARGB.

I'm pretty sure that it's a mistake and it hasn't been an intentional
choice, because even when you would consider a yellow color to for
whatever reason, it wouldn't have been exactly this yellow (#ff0),
because its lightness/saturation values are not in a range of what
is suitable for colored subtitles.

softworkz