Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
@ 2022-01-04  2:19 ffmpegagent
  2022-02-03 22:10 ` Soft Works
  0 siblings, 1 reply; 6+ messages in thread
From: ffmpegagent @ 2022-01-04  2:19 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: softworkz

From: softworkz <softworkz@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@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
  2022-01-04  2:19 [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles ffmpegagent
@ 2022-02-03 22:10 ` Soft Works
  2022-02-03 22:16   ` Scott Theisen
  0 siblings, 1 reply; 6+ messages in thread
From: Soft Works @ 2022-02-03 22:10 UTC (permalink / raw)
  To: ffmpegagent, ffmpeg-devel



> -----Original Message-----
> From: ffmpegagent <ffmpegagent@gmail.com>
> Sent: Tuesday, January 4, 2022 3:19 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: softworkz <softworkz@hotmail.com>; softworkz
> <softworkz@hotmail.com>
> Subject: [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of
> dvd subtitles
> 
> From: softworkz <softworkz@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@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)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
  2022-02-03 22:10 ` Soft Works
@ 2022-02-03 22:16   ` Scott Theisen
  2022-02-03 22:57     ` Soft Works
  0 siblings, 1 reply; 6+ messages in thread
From: Scott Theisen @ 2022-02-03 22:16 UTC (permalink / raw)
  To: ffmpeg-devel

On 2/3/22 17:10, Soft Works wrote:
>
>> -----Original Message-----
>> From: ffmpegagent <ffmpegagent@gmail.com>
>> Sent: Tuesday, January 4, 2022 3:19 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Cc: softworkz <softworkz@hotmail.com>; softworkz
>> <softworkz@hotmail.com>
>> Subject: [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of
>> dvd subtitles
>>
>> From: softworkz <softworkz@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@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/2510a0821ea4453eb9b34dd96e68ff0441459d0b
references: https://code.mythtv.org/trac/ticket/8222
For whatever strange reason, ffmpeg has always used yellow: 
https://github.com/FFmpeg/FFmpeg/commit/240c1657dcd45adc0e63ef947b920919071ec1f7


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
  2022-02-03 22:16   ` Scott Theisen
@ 2022-02-03 22:57     ` Soft Works
  2022-02-03 23:14       ` Scott Theisen
  0 siblings, 1 reply; 6+ messages in thread
From: Soft Works @ 2022-02-03 22:57 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Scott Theisen
> Sent: Thursday, February 3, 2022 11:17 PM
> To: ffmpeg-devel@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@gmail.com>
> >> Sent: Tuesday, January 4, 2022 3:19 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Cc: softworkz <softworkz@hotmail.com>; softworkz
> >> <softworkz@hotmail.com>
> >> Subject: [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance
> of
> >> dvd subtitles
> >>
> >> From: softworkz <softworkz@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@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






_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
  2022-02-03 22:57     ` Soft Works
@ 2022-02-03 23:14       ` Scott Theisen
  2022-02-04  0:05         ` Soft Works
  0 siblings, 1 reply; 6+ messages in thread
From: Scott Theisen @ 2022-02-03 23:14 UTC (permalink / raw)
  To: ffmpeg-devel

On 2/3/22 17:57, Soft Works wrote:
> My suspicion is that 0xffff00 was written assuming RGBA order instead
> of ARGB.

You are missing a byte for either RGBA or ARGB.  RGBA would be cyan.
> 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.

I think using yellow for debugging  purposes is more likely, i.e. it is 
obvious if the pallet was missing/not detected.  However, it shouldn't 
have been committed like that, if that was the reason.

Regards,
Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
  2022-02-03 23:14       ` Scott Theisen
@ 2022-02-04  0:05         ` Soft Works
  0 siblings, 0 replies; 6+ messages in thread
From: Soft Works @ 2022-02-04  0:05 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Scott Theisen
> Sent: Friday, February 4, 2022 12:15 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect
> yellow appearance of dvd subtitles
> 
> On 2/3/22 17:57, Soft Works wrote:
> > My suspicion is that 0xffff00 was written assuming RGBA order
> instead
> > of ARGB.
> 
> You are missing a byte for either RGBA or ARGB.  RGBA would be cyan.
> I think using yellow for debugging  purposes is more likely, i.e. it

Something made me think that it might have come
from a byte ordering issue when I had debugged an example, but 
I don't remember exactly. Probably you are just right about it, but 
only Fabrice might be able to tell for sure..

> However, it shouldn't
> have been committed like that, if that was the reason.

No matter the reason, I don't think there's any doubt that it's wrong?

softworkz

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-04  0:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04  2:19 [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles ffmpegagent
2022-02-03 22:10 ` Soft Works
2022-02-03 22:16   ` Scott Theisen
2022-02-03 22:57     ` Soft Works
2022-02-03 23:14       ` Scott Theisen
2022-02-04  0:05         ` Soft Works

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git