[FFmpeg-devel] [PATCH] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description

[FFmpeg-devel] [PATCH] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description

[Jeffrey Knockel]

Previously the description was partially mistaken, explaining the format
as RGB 3:3:2, (msb)2R 3G 3B(lsb).  While the RGB 3:3:2 part is correct,
the latter part should be: (msb)3R 3G 2B(lsb). The corresponding bit
masks are red: 0xE0, green: 0x1C, blue: 0x03.

Signed-off-by: Jeffrey Knockel <jeff@jeffreyknockel.com>
---
 libavutil/pixfmt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index 58f9ad28bd..9c87571f49 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -83,7 +83,7 @@ enum AVPixelFormat {
     AV_PIX_FMT_BGR8,      ///< packed RGB 3:3:2,  8bpp, (msb)2B 3G 3R(lsb)
     AV_PIX_FMT_BGR4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1B 2G 1R(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
     AV_PIX_FMT_BGR4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1B 2G 1R(lsb)
-    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)2R 3G 3B(lsb)
+    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)3R 3G 2B(lsb)
     AV_PIX_FMT_RGB4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1R 2G 1B(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
     AV_PIX_FMT_RGB4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1R 2G 1B(lsb)
     AV_PIX_FMT_NV12,      ///< planar YUV 4:2:0, 12bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V)
-- 
2.34.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description

[Stefano Sabatini]

On date Monday 2024-01-08 09:53:37 -0500, Jeffrey Knockel wrote:
> Previously the description was partially mistaken, explaining the format
> as RGB 3:3:2, (msb)2R 3G 3B(lsb).  While the RGB 3:3:2 part is correct,
> the latter part should be: (msb)3R 3G 2B(lsb). The corresponding bit
> masks are red: 0xE0, green: 0x1C, blue: 0x03.
> 
> Signed-off-by: Jeffrey Knockel <jeff@jeffreyknockel.com>
> ---
>  libavutil/pixfmt.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> index 58f9ad28bd..9c87571f49 100644
> --- a/libavutil/pixfmt.h
> +++ b/libavutil/pixfmt.h
> @@ -83,7 +83,7 @@ enum AVPixelFormat {
>      AV_PIX_FMT_BGR8,      ///< packed RGB 3:3:2,  8bpp, (msb)2B 3G 3R(lsb)
>      AV_PIX_FMT_BGR4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1B 2G 1R(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
>      AV_PIX_FMT_BGR4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1B 2G 1R(lsb)

> -    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)2R 3G 3B(lsb)
> +    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)3R 3G 2B(lsb)

This is not consistent with pixdesc definition:
    [AV_PIX_FMT_RGB8] = {
        .name = "rgb8",
        .nb_components = 3,
        .log2_chroma_w = 0,
        .log2_chroma_h = 0,
        .comp = {
            { 0, 1, 0, 6, 2 },        /* R */
            { 0, 1, 0, 3, 3 },        /* G */
            { 0, 1, 0, 0, 3 },        /* B */
        },
        .flags = AV_PIX_FMT_FLAG_RGB,
    },

so either the documentation or the pixdesc description is wrong.

Do you confirm "(msb)3R 3G 2B(lsb)" is the correct definition?

In this case pixdesc should be fixed accordingly.

Thanks.
_______________________________________________
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".

[FFmpeg-devel] [PATCH v2] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description

[Jeffrey Knockel]

Previously AV_PIX_FMT_RGB8 was documented as "RGB 3:3:2,
(msb)2R 3G 3B(lsb)".  While the RGB 3:3:2 part is correct, the latter
part should be: (msb)3R 3G 2B(lsb).  This commit also updates the
format's pixdesc description to be (msb)3R 3G 2B(lsb).

Signed-off-by: Jeffrey Knockel <jeff@jeffreyknockel.com>
---
 libavutil/pixdesc.c | 6 +++---
 libavutil/pixfmt.h  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index 0db4167934..f6d4d01460 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -530,9 +530,9 @@ static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
         .log2_chroma_w = 0,
         .log2_chroma_h = 0,
         .comp = {
-            { 0, 1, 0, 6, 2 },        /* R */
-            { 0, 1, 0, 3, 3 },        /* G */
-            { 0, 1, 0, 0, 3 },        /* B */
+            { 0, 1, 0, 5, 3 },        /* R */
+            { 0, 1, 0, 2, 3 },        /* G */
+            { 0, 1, 0, 0, 2 },        /* B */
         },
         .flags = AV_PIX_FMT_FLAG_RGB,
     },
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index 58f9ad28bd..9c87571f49 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -83,7 +83,7 @@ enum AVPixelFormat {
     AV_PIX_FMT_BGR8,      ///< packed RGB 3:3:2,  8bpp, (msb)2B 3G 3R(lsb)
     AV_PIX_FMT_BGR4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1B 2G 1R(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
     AV_PIX_FMT_BGR4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1B 2G 1R(lsb)
-    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)2R 3G 3B(lsb)
+    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)3R 3G 2B(lsb)
     AV_PIX_FMT_RGB4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1R 2G 1B(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
     AV_PIX_FMT_RGB4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1R 2G 1B(lsb)
     AV_PIX_FMT_NV12,      ///< planar YUV 4:2:0, 12bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V)
-- 
2.34.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description

[Jeffrey Knockel]

On 1/14/24 6:32 AM, Stefano Sabatini wrote:
> On date Monday 2024-01-08 09:53:37 -0500, Jeffrey Knockel wrote:
>> Previously the description was partially mistaken, explaining the format
>> as RGB 3:3:2, (msb)2R 3G 3B(lsb).  While the RGB 3:3:2 part is correct,
>> the latter part should be: (msb)3R 3G 2B(lsb). The corresponding bit
>> masks are red: 0xE0, green: 0x1C, blue: 0x03.
>>
>> Signed-off-by: Jeffrey Knockel <jeff@jeffreyknockel.com>
>> ---
>>  libavutil/pixfmt.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
>> index 58f9ad28bd..9c87571f49 100644
>> --- a/libavutil/pixfmt.h
>> +++ b/libavutil/pixfmt.h
>> @@ -83,7 +83,7 @@ enum AVPixelFormat {
>>      AV_PIX_FMT_BGR8,      ///< packed RGB 3:3:2,  8bpp, (msb)2B 3G 3R(lsb)
>>      AV_PIX_FMT_BGR4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1B 2G 1R(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
>>      AV_PIX_FMT_BGR4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1B 2G 1R(lsb)
> 
>> -    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)2R 3G 3B(lsb)
>> +    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)3R 3G 2B(lsb)
> 
> This is not consistent with pixdesc definition:
>     [AV_PIX_FMT_RGB8] = {
>         .name = "rgb8",
>         .nb_components = 3,
>         .log2_chroma_w = 0,
>         .log2_chroma_h = 0,
>         .comp = {
>             { 0, 1, 0, 6, 2 },        /* R */
>             { 0, 1, 0, 3, 3 },        /* G */
>             { 0, 1, 0, 0, 3 },        /* B */
>         },
>         .flags = AV_PIX_FMT_FLAG_RGB,
>     },
> 
> so either the documentation or the pixdesc description is wrong.
> 
> Do you confirm "(msb)3R 3G 2B(lsb)" is the correct definition?

Yes, I created three solid-colored RGBA raw samples filled with either #ff0000ff, #00ff00ff, or #0000ffff, and converted them using `ffmpeg ... -pix_fmt rgba ... -pix_fmt rgb8 ...` and inspected the output.  I also tested with `ffmpeg ... -pix_fmt rgba ... -pix_fmt yuv420p ...` followed by `ffmpeg ... -pix_fmt yuv420p ... -pix_fmt rgb8 ...`.  In these tests the outputs were consistent with "(msb)3R 3G 2B(lsb)".

> In this case pixdesc should be fixed accordingly.

Got it, will send an updated patch!

Thanks,
Jeff
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description

[Diederick C. Niehorster]

On Sun, Jan 14, 2024 at 4:15 PM Jeffrey Knockel <jeff@jeffreyknockel.com> wrote:
>
> Previously AV_PIX_FMT_RGB8 was documented as "RGB 3:3:2,
> (msb)2R 3G 3B(lsb)".  While the RGB 3:3:2 part is correct, the latter
> part should be: (msb)3R 3G 2B(lsb).  This commit also updates the
> format's pixdesc description to be (msb)3R 3G 2B(lsb).
>
> Signed-off-by: Jeffrey Knockel <jeff@jeffreyknockel.com>
> ---
>  libavutil/pixdesc.c | 6 +++---
>  libavutil/pixfmt.h  | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index 0db4167934..f6d4d01460 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -530,9 +530,9 @@ static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
>          .log2_chroma_w = 0,
>          .log2_chroma_h = 0,
>          .comp = {
> -            { 0, 1, 0, 6, 2 },        /* R */
> -            { 0, 1, 0, 3, 3 },        /* G */
> -            { 0, 1, 0, 0, 3 },        /* B */
> +            { 0, 1, 0, 5, 3 },        /* R */
> +            { 0, 1, 0, 2, 3 },        /* G */
> +            { 0, 1, 0, 0, 2 },        /* B */
>          },
>          .flags = AV_PIX_FMT_FLAG_RGB,
>      },
> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> index 58f9ad28bd..9c87571f49 100644
> --- a/libavutil/pixfmt.h
> +++ b/libavutil/pixfmt.h
> @@ -83,7 +83,7 @@ enum AVPixelFormat {
>      AV_PIX_FMT_BGR8,      ///< packed RGB 3:3:2,  8bpp, (msb)2B 3G 3R(lsb)
>      AV_PIX_FMT_BGR4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1B 2G 1R(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
>      AV_PIX_FMT_BGR4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1B 2G 1R(lsb)
> -    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)2R 3G 3B(lsb)
> +    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)3R 3G 2B(lsb)
>      AV_PIX_FMT_RGB4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1R 2G 1B(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
>      AV_PIX_FMT_RGB4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1R 2G 1B(lsb)
>      AV_PIX_FMT_NV12,      ///< planar YUV 4:2:0, 12bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V)
> --
> 2.34.1

LGTM.
Tested. Without this patch, the output of avutil av_read_image_line2()
is all wrong, with it applied it is correct.

All the best,
Dee
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2882 bytes --]

On Sun, Jan 14, 2024 at 09:58:52PM +0100, Diederick C. Niehorster wrote:
> On Sun, Jan 14, 2024 at 4:15 PM Jeffrey Knockel <jeff@jeffreyknockel.com> wrote:
> >
> > Previously AV_PIX_FMT_RGB8 was documented as "RGB 3:3:2,
> > (msb)2R 3G 3B(lsb)".  While the RGB 3:3:2 part is correct, the latter
> > part should be: (msb)3R 3G 2B(lsb).  This commit also updates the
> > format's pixdesc description to be (msb)3R 3G 2B(lsb).
> >
> > Signed-off-by: Jeffrey Knockel <jeff@jeffreyknockel.com>
> > ---
> >  libavutil/pixdesc.c | 6 +++---
> >  libavutil/pixfmt.h  | 2 +-
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> > index 0db4167934..f6d4d01460 100644
> > --- a/libavutil/pixdesc.c
> > +++ b/libavutil/pixdesc.c
> > @@ -530,9 +530,9 @@ static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
> >          .log2_chroma_w = 0,
> >          .log2_chroma_h = 0,
> >          .comp = {
> > -            { 0, 1, 0, 6, 2 },        /* R */
> > -            { 0, 1, 0, 3, 3 },        /* G */
> > -            { 0, 1, 0, 0, 3 },        /* B */
> > +            { 0, 1, 0, 5, 3 },        /* R */
> > +            { 0, 1, 0, 2, 3 },        /* G */
> > +            { 0, 1, 0, 0, 2 },        /* B */
> >          },
> >          .flags = AV_PIX_FMT_FLAG_RGB,
> >      },
> > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> > index 58f9ad28bd..9c87571f49 100644
> > --- a/libavutil/pixfmt.h
> > +++ b/libavutil/pixfmt.h
> > @@ -83,7 +83,7 @@ enum AVPixelFormat {
> >      AV_PIX_FMT_BGR8,      ///< packed RGB 3:3:2,  8bpp, (msb)2B 3G 3R(lsb)
> >      AV_PIX_FMT_BGR4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1B 2G 1R(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
> >      AV_PIX_FMT_BGR4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1B 2G 1R(lsb)
> > -    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)2R 3G 3B(lsb)
> > +    AV_PIX_FMT_RGB8,      ///< packed RGB 3:3:2,  8bpp, (msb)3R 3G 2B(lsb)
> >      AV_PIX_FMT_RGB4,      ///< packed RGB 1:2:1 bitstream,  4bpp, (msb)1R 2G 1B(lsb), a byte contains two pixels, the first pixel in the byte is the one composed by the 4 msb bits
> >      AV_PIX_FMT_RGB4_BYTE, ///< packed RGB 1:2:1,  8bpp, (msb)1R 2G 1B(lsb)
> >      AV_PIX_FMT_NV12,      ///< planar YUV 4:2:0, 12bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V)
> > --
> > 2.34.1
> 
> LGTM.
> Tested. Without this patch, the output of avutil av_read_image_line2()
> is all wrong, with it applied it is correct.

will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".