[FFmpeg-devel,2/2] pixdesc: deprecate AV_PIX_FMT_FLAG_PSEUDOPAL

Submitted by wm4 on March 29, 2018, 1:30 p.m.

Details

Message ID 20180329133043.21890-2-nfxjfg@googlemail.com
State New
Headers show

Commit Message

wm4 March 29, 2018, 1:30 p.m.
PSEUDOPAL pixel formats are not paletted, but carried a palette with the
intention of allowing code to treat unpaletted formats as paletted. The
palette simply mapped the byte values to the resulting RGB values,
making it some sort of LUT for RGB conversion.

It was used for 1 byte formats only: RGB4_BYTE, BGR4_BYTE, RGB8, BGR8,
GRAY8. The first 4 are awfully obscure, used only by some ancient bitmap
formats. The last one, GRAY8, is more common, but its treatment is
grossly incorrect. It considers full range GRAY8 only, so GRAY8 coming
from typical Y video planes was not mapped to the correct RGB values.
Also, nothing actually used the PSEUDOPAL palette data, except xwdenc.
All other code had to treat it as a special case, just to ignore or
propagate palette data.

In conclusion, this was just a very strange old hack that has no real
justification to exist. It's negatively useful, because API users who
allocate their own pixel data have to be aware that they need to
allocate the palette, or FFmpeg will crash on them in _some_ situations.
On top of this, there was no API to allocate the pseuo palette outside
of av_frame_get_buffer(). (avpriv_set_systematic_pal2() was not public,
which is good, because GRAY8 treatment is broken.)

This patch not only deprecates AV_PIX_FMT_FLAG_PSEUDOPAL, but also makes
the pseudo palette optional. Nothing accesses it anymore, though if it's
set, it's propagated. It's still allocated and initialized for
compatibility with API users that rely on this feature. But new API
users do not need to allocate it. This was an explicit goal of this
patch.

Most changes replace AV_PIX_FMT_FLAG_PSEUDOPAL with FF_PSEUDOPAL. I
first tried #ifdefing all code, but it was a mess. The FF_PSEUDOPAL
macro reduces the mess, and still allows defining FF_API_PSEUDOPAL to 0.

Passes FATE with FF_API_PSEUDOPAL enabled and disabled. In addition,
FATE passes with FF_API_PSEUDOPAL set to 1, but with allocation
functions manually changed to not allocating a palette.
---
 fftools/ffprobe.c              |  2 ++
 libavcodec/decode.c            |  5 +----
 libavcodec/ffv1dec.c           |  2 +-
 libavcodec/rawdec.c            |  6 ++++--
 libavcodec/smvjpegdec.c        |  2 +-
 libavfilter/drawutils.c        |  2 +-
 libavfilter/framepool.c        |  4 ++--
 libavfilter/vf_crop.c          |  2 +-
 libavfilter/vf_pixdesctest.c   |  2 +-
 libavfilter/vf_scale.c         |  5 ++---
 libavfilter/vf_shuffleplanes.c |  2 +-
 libavutil/frame.c              |  4 ++--
 libavutil/imgutils.c           | 13 +++++++------
 libavutil/internal.h           |  9 +++++++++
 libavutil/pixdesc.c            | 10 +++++-----
 libavutil/pixdesc.h            |  8 ++++++++
 libavutil/version.h            |  3 +++
 libswscale/swscale_internal.h  | 14 +++++++++++---
 18 files changed, 62 insertions(+), 33 deletions(-)

Comments

Michael Niedermayer March 30, 2018, 1:13 a.m.
On Thu, Mar 29, 2018 at 03:30:43PM +0200, wm4 wrote:
> PSEUDOPAL pixel formats are not paletted, but carried a palette with the
> intention of allowing code to treat unpaletted formats as paletted. The
> palette simply mapped the byte values to the resulting RGB values,
> making it some sort of LUT for RGB conversion.
> 
> It was used for 1 byte formats only: RGB4_BYTE, BGR4_BYTE, RGB8, BGR8,
> GRAY8. The first 4 are awfully obscure, used only by some ancient bitmap
> formats. The last one, GRAY8, is more common, but its treatment is
> grossly incorrect. It considers full range GRAY8 only, so GRAY8 coming
> from typical Y video planes was not mapped to the correct RGB values.
> Also, nothing actually used the PSEUDOPAL palette data, except xwdenc.
> All other code had to treat it as a special case, just to ignore or
> propagate palette data.
> 
> In conclusion, this was just a very strange old hack that has no real
> justification to exist. It's negatively useful, because API users who
> allocate their own pixel data have to be aware that they need to
> allocate the palette, or FFmpeg will crash on them in _some_ situations.
> On top of this, there was no API to allocate the pseuo palette outside
> of av_frame_get_buffer(). (avpriv_set_systematic_pal2() was not public,
> which is good, because GRAY8 treatment is broken.)
> 
> This patch not only deprecates AV_PIX_FMT_FLAG_PSEUDOPAL, but also makes
> the pseudo palette optional. Nothing accesses it anymore, though if it's
> set, it's propagated. It's still allocated and initialized for
> compatibility with API users that rely on this feature. But new API
> users do not need to allocate it. This was an explicit goal of this
> patch.
> 
> Most changes replace AV_PIX_FMT_FLAG_PSEUDOPAL with FF_PSEUDOPAL. I
> first tried #ifdefing all code, but it was a mess. The FF_PSEUDOPAL
> macro reduces the mess, and still allows defining FF_API_PSEUDOPAL to 0.
> 
> Passes FATE with FF_API_PSEUDOPAL enabled and disabled. In addition,
> FATE passes with FF_API_PSEUDOPAL set to 1, but with allocation
> functions manually changed to not allocating a palette.
> ---

iam not sure if your rants / political views belong in a commit message.
I think they should be removed. 

about the patch, ive not tested it yet or looked deeper but
please seperate the identifer renaming out into its own patch, that way
it will be much more readable.


thx

[...]
wm4 March 30, 2018, 1:23 a.m.
On Fri, 30 Mar 2018 03:13:07 +0200
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Thu, Mar 29, 2018 at 03:30:43PM +0200, wm4 wrote:
> > PSEUDOPAL pixel formats are not paletted, but carried a palette with the
> > intention of allowing code to treat unpaletted formats as paletted. The
> > palette simply mapped the byte values to the resulting RGB values,
> > making it some sort of LUT for RGB conversion.
> > 
> > It was used for 1 byte formats only: RGB4_BYTE, BGR4_BYTE, RGB8, BGR8,
> > GRAY8. The first 4 are awfully obscure, used only by some ancient bitmap
> > formats. The last one, GRAY8, is more common, but its treatment is
> > grossly incorrect. It considers full range GRAY8 only, so GRAY8 coming
> > from typical Y video planes was not mapped to the correct RGB values.
> > Also, nothing actually used the PSEUDOPAL palette data, except xwdenc.
> > All other code had to treat it as a special case, just to ignore or
> > propagate palette data.
> > 
> > In conclusion, this was just a very strange old hack that has no real
> > justification to exist. It's negatively useful, because API users who
> > allocate their own pixel data have to be aware that they need to
> > allocate the palette, or FFmpeg will crash on them in _some_ situations.
> > On top of this, there was no API to allocate the pseuo palette outside
> > of av_frame_get_buffer(). (avpriv_set_systematic_pal2() was not public,
> > which is good, because GRAY8 treatment is broken.)
> > 
> > This patch not only deprecates AV_PIX_FMT_FLAG_PSEUDOPAL, but also makes
> > the pseudo palette optional. Nothing accesses it anymore, though if it's
> > set, it's propagated. It's still allocated and initialized for
> > compatibility with API users that rely on this feature. But new API
> > users do not need to allocate it. This was an explicit goal of this
> > patch.
> > 
> > Most changes replace AV_PIX_FMT_FLAG_PSEUDOPAL with FF_PSEUDOPAL. I
> > first tried #ifdefing all code, but it was a mess. The FF_PSEUDOPAL
> > macro reduces the mess, and still allows defining FF_API_PSEUDOPAL to 0.
> > 
> > Passes FATE with FF_API_PSEUDOPAL enabled and disabled. In addition,
> > FATE passes with FF_API_PSEUDOPAL set to 1, but with allocation
> > functions manually changed to not allocating a palette.
> > ---  
> 
> iam not sure if your rants / political views belong in a commit message.
> I think they should be removed. 

There are no "political" views. Please point out which parts you think
are political, and why they supposedly are political.

There are no rants either. In fact, calling them rants is disrespectful
and implies there is no logic behind whatever parts you think are rants.

> about the patch, ive not tested it yet or looked deeper but
> please seperate the identifer renaming out into its own patch, that way
> it will be much more readable.

There's nothing being renamed. This is the deprecation. Do you prefer if
I litter the code with ifdefs instead? Did you read the commit message?

> 
> thx

If you meant it you'd do a proper review.
Michael Niedermayer March 30, 2018, 11:22 a.m.
On Fri, Mar 30, 2018 at 03:23:25AM +0200, wm4 wrote:
> On Fri, 30 Mar 2018 03:13:07 +0200
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> > On Thu, Mar 29, 2018 at 03:30:43PM +0200, wm4 wrote:
> > > PSEUDOPAL pixel formats are not paletted, but carried a palette with the
> > > intention of allowing code to treat unpaletted formats as paletted. The
> > > palette simply mapped the byte values to the resulting RGB values,
> > > making it some sort of LUT for RGB conversion.
> > > 
> > > It was used for 1 byte formats only: RGB4_BYTE, BGR4_BYTE, RGB8, BGR8,
> > > GRAY8. The first 4 are awfully obscure, used only by some ancient bitmap
> > > formats. The last one, GRAY8, is more common, but its treatment is
> > > grossly incorrect. It considers full range GRAY8 only, so GRAY8 coming
> > > from typical Y video planes was not mapped to the correct RGB values.
> > > Also, nothing actually used the PSEUDOPAL palette data, except xwdenc.
> > > All other code had to treat it as a special case, just to ignore or
> > > propagate palette data.
> > > 
> > > In conclusion, this was just a very strange old hack that has no real
> > > justification to exist. It's negatively useful, because API users who
> > > allocate their own pixel data have to be aware that they need to
> > > allocate the palette, or FFmpeg will crash on them in _some_ situations.
> > > On top of this, there was no API to allocate the pseuo palette outside
> > > of av_frame_get_buffer(). (avpriv_set_systematic_pal2() was not public,
> > > which is good, because GRAY8 treatment is broken.)
> > > 
> > > This patch not only deprecates AV_PIX_FMT_FLAG_PSEUDOPAL, but also makes
> > > the pseudo palette optional. Nothing accesses it anymore, though if it's
> > > set, it's propagated. It's still allocated and initialized for
> > > compatibility with API users that rely on this feature. But new API
> > > users do not need to allocate it. This was an explicit goal of this
> > > patch.
> > > 
> > > Most changes replace AV_PIX_FMT_FLAG_PSEUDOPAL with FF_PSEUDOPAL. I
> > > first tried #ifdefing all code, but it was a mess. The FF_PSEUDOPAL
> > > macro reduces the mess, and still allows defining FF_API_PSEUDOPAL to 0.
> > > 
> > > Passes FATE with FF_API_PSEUDOPAL enabled and disabled. In addition,
> > > FATE passes with FF_API_PSEUDOPAL set to 1, but with allocation
> > > functions manually changed to not allocating a palette.
> > > ---  
> > 
> > iam not sure if your rants / political views belong in a commit message.
> > I think they should be removed. 
> 
> There are no "political" views. Please point out which parts you think
> are political, and why they supposedly are political.
> 
> There are no rants either. In fact, calling them rants is disrespectful
> and implies there is no logic behind whatever parts you think are rants.

you made disrespectful comments in the commit message above.
That is implying code others have written is
"this was just a very strange old hack that has no real justification to exist"
"It's negatively useful"

iam not sure if you are trying to pick a fight or what the point of these is
but

These are clearly not technical comments nor are they correct.
The code had sense, was usefull, and allowed other code to be simplified.

I called them political rants, maybe they are something else but either way
i do not belive this belongs in a commit message.



> 
> > about the patch, ive not tested it yet or looked deeper but
> > please seperate the identifer renaming out into its own patch, that way
> > it will be much more readable.
> 
> There's nothing being renamed. 

Of course there is, the whole patch is full of changes like this:

@@ -423,7 +425,7 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
     }

     if ((avctx->pix_fmt == AV_PIX_FMT_PAL8 && buf_size < context->frame_size) ||
-        (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)) {
+        (desc->flags & FF_PSEUDOPAL)) {
         frame->buf[1]  = av_buffer_ref(context->palette);
         if (!frame->buf[1]) {
             av_buffer_unref(&frame->buf[0]);

Here above its clearly vissible but in many hunks its intermixed with other
changes making the patch hard to read

             
> This is the deprecation. Do you prefer if
> I litter the code with ifdefs instead? Did you read the commit message?

I did read the commit message, otherwise how could i complain about it?


> 
> > 
> > thx
> 
> If you meant it you'd do a proper review.

Please stop with these disrespectful snide comments.
I intended to review it once the renaming is split out.

thank you

[...]
Paul B Mahol March 30, 2018, 11:31 a.m.
On 3/30/18, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Mar 30, 2018 at 03:23:25AM +0200, wm4 wrote:
>> On Fri, 30 Mar 2018 03:13:07 +0200
>> Michael Niedermayer <michael@niedermayer.cc> wrote:
>>
>> > On Thu, Mar 29, 2018 at 03:30:43PM +0200, wm4 wrote:
>> > > PSEUDOPAL pixel formats are not paletted, but carried a palette with
>> > > the
>> > > intention of allowing code to treat unpaletted formats as paletted.
>> > > The
>> > > palette simply mapped the byte values to the resulting RGB values,
>> > > making it some sort of LUT for RGB conversion.
>> > >
>> > > It was used for 1 byte formats only: RGB4_BYTE, BGR4_BYTE, RGB8,
>> > > BGR8,
>> > > GRAY8. The first 4 are awfully obscure, used only by some ancient
>> > > bitmap
>> > > formats. The last one, GRAY8, is more common, but its treatment is
>> > > grossly incorrect. It considers full range GRAY8 only, so GRAY8
>> > > coming
>> > > from typical Y video planes was not mapped to the correct RGB values.
>> > > Also, nothing actually used the PSEUDOPAL palette data, except
>> > > xwdenc.
>> > > All other code had to treat it as a special case, just to ignore or
>> > > propagate palette data.
>> > >
>> > > In conclusion, this was just a very strange old hack that has no real
>> > > justification to exist. It's negatively useful, because API users who
>> > > allocate their own pixel data have to be aware that they need to
>> > > allocate the palette, or FFmpeg will crash on them in _some_
>> > > situations.
>> > > On top of this, there was no API to allocate the pseuo palette
>> > > outside
>> > > of av_frame_get_buffer(). (avpriv_set_systematic_pal2() was not
>> > > public,
>> > > which is good, because GRAY8 treatment is broken.)
>> > >
>> > > This patch not only deprecates AV_PIX_FMT_FLAG_PSEUDOPAL, but also
>> > > makes
>> > > the pseudo palette optional. Nothing accesses it anymore, though if
>> > > it's
>> > > set, it's propagated. It's still allocated and initialized for
>> > > compatibility with API users that rely on this feature. But new API
>> > > users do not need to allocate it. This was an explicit goal of this
>> > > patch.
>> > >
>> > > Most changes replace AV_PIX_FMT_FLAG_PSEUDOPAL with FF_PSEUDOPAL. I
>> > > first tried #ifdefing all code, but it was a mess. The FF_PSEUDOPAL
>> > > macro reduces the mess, and still allows defining FF_API_PSEUDOPAL to
>> > > 0.
>> > >
>> > > Passes FATE with FF_API_PSEUDOPAL enabled and disabled. In addition,
>> > > FATE passes with FF_API_PSEUDOPAL set to 1, but with allocation
>> > > functions manually changed to not allocating a palette.
>> > > ---
>> >
>> > iam not sure if your rants / political views belong in a commit
>> > message.
>> > I think they should be removed.
>>
>> There are no "political" views. Please point out which parts you think
>> are political, and why they supposedly are political.
>>
>> There are no rants either. In fact, calling them rants is disrespectful
>> and implies there is no logic behind whatever parts you think are rants.
>
> you made disrespectful comments in the commit message above.
> That is implying code others have written is
> "this was just a very strange old hack that has no real justification to
> exist"
> "It's negatively useful"
>
> iam not sure if you are trying to pick a fight or what the point of these
> is
> but
>
> These are clearly not technical comments nor are they correct.
> The code had sense, was usefull, and allowed other code to be simplified.
>
> I called them political rants, maybe they are something else but either way
> i do not belive this belongs in a commit message.
>
>
>
>>
>> > about the patch, ive not tested it yet or looked deeper but
>> > please seperate the identifer renaming out into its own patch, that way
>> > it will be much more readable.
>>
>> There's nothing being renamed.
>
> Of course there is, the whole patch is full of changes like this:
>
> @@ -423,7 +425,7 @@ static int raw_decode(AVCodecContext *avctx, void *data,
> int *got_frame,
>      }
>
>      if ((avctx->pix_fmt == AV_PIX_FMT_PAL8 && buf_size <
> context->frame_size) ||
> -        (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)) {
> +        (desc->flags & FF_PSEUDOPAL)) {
>          frame->buf[1]  = av_buffer_ref(context->palette);
>          if (!frame->buf[1]) {
>              av_buffer_unref(&frame->buf[0]);
>
> Here above its clearly vissible but in many hunks its intermixed with other
> changes making the patch hard to read
>
>
>> This is the deprecation. Do you prefer if
>> I litter the code with ifdefs instead? Did you read the commit message?
>
> I did read the commit message, otherwise how could i complain about it?
>
>
>>
>> >
>> > thx
>>
>> If you meant it you'd do a proper review.
>
> Please stop with these disrespectful snide comments.
> I intended to review it once the renaming is split out.
>
> thank you

Why you and some other 'old' developers have urge to block every single patch
that comes from some developers?
wm4 March 30, 2018, 1:10 p.m.
On Fri, 30 Mar 2018 13:22:25 +0200
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Fri, Mar 30, 2018 at 03:23:25AM +0200, wm4 wrote:
> > On Fri, 30 Mar 2018 03:13:07 +0200
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >   
> > > On Thu, Mar 29, 2018 at 03:30:43PM +0200, wm4 wrote:  
> > > > PSEUDOPAL pixel formats are not paletted, but carried a palette with the
> > > > intention of allowing code to treat unpaletted formats as paletted. The
> > > > palette simply mapped the byte values to the resulting RGB values,
> > > > making it some sort of LUT for RGB conversion.
> > > > 
> > > > It was used for 1 byte formats only: RGB4_BYTE, BGR4_BYTE, RGB8, BGR8,
> > > > GRAY8. The first 4 are awfully obscure, used only by some ancient bitmap
> > > > formats. The last one, GRAY8, is more common, but its treatment is
> > > > grossly incorrect. It considers full range GRAY8 only, so GRAY8 coming
> > > > from typical Y video planes was not mapped to the correct RGB values.
> > > > Also, nothing actually used the PSEUDOPAL palette data, except xwdenc.
> > > > All other code had to treat it as a special case, just to ignore or
> > > > propagate palette data.
> > > > 
> > > > In conclusion, this was just a very strange old hack that has no real
> > > > justification to exist. It's negatively useful, because API users who
> > > > allocate their own pixel data have to be aware that they need to
> > > > allocate the palette, or FFmpeg will crash on them in _some_ situations.
> > > > On top of this, there was no API to allocate the pseuo palette outside
> > > > of av_frame_get_buffer(). (avpriv_set_systematic_pal2() was not public,
> > > > which is good, because GRAY8 treatment is broken.)
> > > > 
> > > > This patch not only deprecates AV_PIX_FMT_FLAG_PSEUDOPAL, but also makes
> > > > the pseudo palette optional. Nothing accesses it anymore, though if it's
> > > > set, it's propagated. It's still allocated and initialized for
> > > > compatibility with API users that rely on this feature. But new API
> > > > users do not need to allocate it. This was an explicit goal of this
> > > > patch.
> > > > 
> > > > Most changes replace AV_PIX_FMT_FLAG_PSEUDOPAL with FF_PSEUDOPAL. I
> > > > first tried #ifdefing all code, but it was a mess. The FF_PSEUDOPAL
> > > > macro reduces the mess, and still allows defining FF_API_PSEUDOPAL to 0.
> > > > 
> > > > Passes FATE with FF_API_PSEUDOPAL enabled and disabled. In addition,
> > > > FATE passes with FF_API_PSEUDOPAL set to 1, but with allocation
> > > > functions manually changed to not allocating a palette.
> > > > ---    
> > > 
> > > iam not sure if your rants / political views belong in a commit message.
> > > I think they should be removed.   
> > 
> > There are no "political" views. Please point out which parts you think
> > are political, and why they supposedly are political.
> > 
> > There are no rants either. In fact, calling them rants is disrespectful
> > and implies there is no logic behind whatever parts you think are rants.  
> 
> you made disrespectful comments in the commit message above.
> That is implying code others have written is
> "this was just a very strange old hack that has no real justification to exist"
> "It's negatively useful"

This is because you take this personally for some reason.

This thing might have had a justification in the old days, when these 1
byte formats made up a significant part of all pixfmts, and handling
some as simply paletted could possibly have simplified code.

You have to admit that carrying a static colorspace conversion LUT
(just to RGB) as a palette as additional AVFrame data pointer is a bit
strange, and could also be called a hack. Let me know if you disagree.

My point is that this mechanism hasn't well aged, and any
expectations the original author might have had from this weren't
fulfilled (or at least not anymore in recent times). Indeed, there was
no code in FFmpeg that even relied on this, except xwdenc (which in
itself has a questionable usefulness, because it's an encoder for the
mostly-raw format of a certain X11 screenshot utility that probably
nobody uses anymore - reading is already a stretch, but encoding is
likely not to be used by anyone).

In the newest time, API users are surprised by this pseudopal
mechanism, and ran into it only because _some_ FFmpeg code crashed in
AVFrame with pseudopal formats that didn't have a palette (source: it
happened to me). As such, it doesn't really help anyone (see above),
but actually causes more work to some (see this paragraph). So I think
"negatively useful" is a justified expression and is not disrespectful
to anyone. (Who would I even address? I don't even know who the
original author was. I just looked and it's actually hard to determine.
I only see a commit by a Libav developer in 2012, which added the flag
to complement the already existing mechanism. As far as disrespect
goes, there is/has been plenty of _actual_ disrespect against Libav
developers in the FFmpeg, and you never did anything against it when
you were still project leader.)

Calling my detailed commit message that intends to present the full
situation in a complete way a "rant" is really very insulting. Do you
think I'm some sort of clown who writes commit messages for fun and to
annoy people?

Why do you try to incite hostilities and discord?

It could have been a technical discussion, but you start things like
this.

> iam not sure if you are trying to pick a fight or what the point of these is
> but
> 
> These are clearly not technical comments nor are they correct.
> The code had sense, was usefull, and allowed other code to be simplified.

Yeah, I see past tense. Like I pointed out above, there was only xwdenc
which used this, and it was trivially replaced. All other code was only
propagating the palette or making space for allocation.

> I called them political rants, maybe they are something else but either way
> i do not belive this belongs in a commit message.

No, this definitely belongs into a commit message:
- What mechanism is changed?
- Why was the mechanism useful?
- Why is the mechanism not useful anymore?
- Why should it be changed at all?

I listed all these things. It's not a "rant". It's hilarious how you
always complain that I'm supposedly disrespectful to you, but then you
always do things like this.

> 
> 
> 
> >   
> > > about the patch, ive not tested it yet or looked deeper but
> > > please seperate the identifer renaming out into its own patch, that way
> > > it will be much more readable.  
> > 
> > There's nothing being renamed.   
> 
> Of course there is, the whole patch is full of changes like this:
> 
> @@ -423,7 +425,7 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
>      }
> 
>      if ((avctx->pix_fmt == AV_PIX_FMT_PAL8 && buf_size < context->frame_size) ||
> -        (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)) {
> +        (desc->flags & FF_PSEUDOPAL)) {
>          frame->buf[1]  = av_buffer_ref(context->palette);
>          if (!frame->buf[1]) {
>              av_buffer_unref(&frame->buf[0]);
> 
> Here above its clearly vissible but in many hunks its intermixed with other
> changes making the patch hard to read

Have you read my commit message? The alternative is littering the code
with #ifdefs which is even less readable. This is due to the FFmpeg
policy that deprecated code needs to be conditionally compiled, based
on FF_API_ flags. I don't think it's appropriate to move this into a
separate patch, just to reduce the size of the patch.

Surely you will be able to read and understand it anyway?

Should we discuss how unreadable the merge commits are that you made for
years?

>              
> > This is the deprecation. Do you prefer if
> > I litter the code with ifdefs instead? Did you read the commit message?  
> 
> I did read the commit message, otherwise how could i complain about it?

By reading it selectively and ignoring other parts.

> 
> >   
> > > 
> > > thx  
> > 
> > If you meant it you'd do a proper review.  
> 
> Please stop with these disrespectful snide comments.

Same to you.

> I intended to review it once the renaming is split out.

I don't intend to make such strange changes. If you don't have anything
actual to contribute, I will push this patch on monday.
Michael Niedermayer March 30, 2018, 11:55 p.m.
On Fri, Mar 30, 2018 at 01:31:35PM +0200, Paul B Mahol wrote:
[...]
> Why you and some other 'old' developers have urge to block every single patch
> that comes from some developers?

Thats a pretty serious accusation, you should not throw this around
lightly unless its true.

Also i would generally prefer not to talk about people in relation to
who did what as its alienating and this is bad for us as a team.
But being accused of something i did not do, how could i defend myself
without refering to what was actually said and by whom. And these
accusations keep comming up ...

Whos patches are being blocked ?
I assume by "some developers" you mean wm4 ? If so lets look at this
case

wm4 blocked tobias av log patch yesterday (Re: [FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function)
    "I'd like to block it, because I don't see it as a good thing that more
     fftools specific stuff is leaking into the generic libs. Sorry."
     
and in the "[FFmpeg-devel] [RFC] avpriv cleanup" thread 3 days before 
    the debate went in circles until i gave up as it just got too much
    effort for the small gain of the change i wanted to do.
    Not technically blocked no
    
and in this thread here with the roles flipped around his comment was
    "I don't intend to make such strange changes. If you don't have anything
     actual to contribute, I will push this patch on monday."
     
He also made similar comments to nicolas. but lets stay with this here

The "strange changes" here are a request to split out renaming an identifer
that is required by our development policy, and everyone else splits their
renamings out too and i do not remember any other developer questioning that
such changes improve the git log readability

you can check https://ffmpeg.org/developer.html
"Cosmetic changes should be kept in separate patches."

And which patch do you belive iam blocking ? I was not intending of blocking any
patch. 

Its really odd, if i look at just the last few days, who blocked patches
and who got accused of it. It just makes no sense. There is not much overlap
between who blocked patches and who got accused of blocking patches.

Either way these hostilities we have here are not good, please calm down, iam
not blocking any patches. If some other developer, wm4, nicolas or whoever 
does, iam sure he has a reason for it, even if someone disagrees. 
The best for such a case would be
to calmly and respectfully discuss. Escalation will likely not achive anything
except Escalation.

Also lets stop accusing people, of things. This just leads to a back and forth
that is harmfull to the team. 
If you think iam doing something bad or wrong, talk with me about it please
theres private mail there is IRC. Hasnt it always been a misunderstanding of
some form in the past?
I dont block patches generally ... If a patch is really bad many people will
object to it, it doesnt require me beyond maybe pointing to the issue to make
sure others are aware of it ...

Peace

[...]
wm4 March 31, 2018, 12:55 a.m.
On Sat, 31 Mar 2018 01:55:52 +0200
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Fri, Mar 30, 2018 at 01:31:35PM +0200, Paul B Mahol wrote:
> [...]
> > Why you and some other 'old' developers have urge to block every single patch
> > that comes from some developers?  
> 
> Thats a pretty serious accusation, you should not throw this around
> lightly unless its true.
> 
> Also i would generally prefer not to talk about people in relation to
> who did what as its alienating and this is bad for us as a team.
> But being accused of something i did not do, how could i defend myself
> without refering to what was actually said and by whom. And these
> accusations keep comming up ...

Maybe they're true at least to some extent? Have you ever thought about
that. Others complained too. Could also list Libav as argument.

I won't claim I'm 100% innocent (just like I don't claim you're 100%
evil). But there is definitely something wrong here with how you
present things.

> Whos patches are being blocked ?
> I assume by "some developers" you mean wm4 ? If so lets look at this
> case

Oh, so you're singling me out as the evil one? That's of course a
direct attack against me. Also since bystanders will trust your word
more than mine (since you're universally known as "the" FFmpeg guy),
this is pretty a pretty serious attack against me.

Anyway, my patches have been blocked or bikeshedded in the past as well
by certain people (including yourself). Blocking/rejecting patches is
part of the normal development process. (What matters is _how_ it
happens and the social aspects around it.)

> wm4 blocked tobias av log patch yesterday (Re: [FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function)
>     "I'd like to block it, because I don't see it as a good thing that more
>      fftools specific stuff is leaking into the generic libs. Sorry."

I'd rather say: you blocked Tobias' initial patch, even though the patch
was completely fine. I was not even aware you objected to me blocking
the second version of the patch. You didn't reply to my arguments, or
reply at all.

> and in the "[FFmpeg-devel] [RFC] avpriv cleanup" thread 3 days before 
>     the debate went in circles until i gave up as it just got too much
>     effort for the small gain of the change i wanted to do.
>     Not technically blocked no

Well, I also spend a lot of time going in circles with you, instead of
making progress. Like in this thread right now.

Can you explain how I "blocked" this? I explicitly encouraged you to
post concrete proposals (though you never did that), and my only gripe
with this was that the discussion was overly broad, with some danger
that you'd just make all avpriv functions public or so.

I'm actually not sure what you're complaining about wrt. to that
avpriv thread. This is how an overly broad discussion will go. So why
did you start one. What was your desired or expected outcome of this
thread? Everyone agrees and then does nothing, because there couldn't
really be any actionable result?

Actually, my patch makes it unnecessary to make one of those avpriv_
functions public, so you could say I contributed to this in a
productive way.

> and in this thread here with the roles flipped around his comment was
>     "I don't intend to make such strange changes. If you don't have anything
>      actual to contribute, I will push this patch on monday."
>      
> He also made similar comments to nicolas. but lets stay with this here

Nicolas George? That guy who aggressively calls me a troll etc. and
even refuses to discuss anything directly with me?

> The "strange changes" here are a request to split out renaming an identifer
> that is required by our development policy, and everyone else splits their
> renamings out too and i do not remember any other developer questioning that
> such changes improve the git log readability
> 
> you can check https://ffmpeg.org/developer.html
> "Cosmetic changes should be kept in separate patches."

These changes are inherent, functional parts of the patch. It would
make little sense to split them out. It'd just obfuscate the
consequences of the change. I explained this, but you didn't really
respond to it and just repeated your original request.

The policy is NOT to split changes in nonsense ways just to reduce
commit sizes. Asking to split patches just for the hell of it is pretty
strange.

> And which patch do you belive iam blocking ? I was not intending of blocking any
> patch. 

You JUST complained that I wanted to push this patch as is.

So what is it:
a) my patch is not blocked, you're content with my reply that I don't
   want to split out those changes we talked about to a separate commit
b) my patch is blocked until I send a new split version of the patch
c) something else which you haven't said yet or something equally vague
   that you intend to complain about after I push the patch

I suspect it's actually c).

> Its really odd, if i look at just the last few days, who blocked patches
> and who got accused of it. It just makes no sense. There is not much overlap
> between who blocked patches and who got accused of blocking patches.
> 
> Either way these hostilities we have here are not good, please calm down, iam
> not blocking any patches. If some other developer, wm4, nicolas or whoever 
> does, iam sure he has a reason for it, even if someone disagrees. 
> The best for such a case would be
> to calmly and respectfully discuss. Escalation will likely not achive anything
> except Escalation.
> 
> Also lets stop accusing people, of things. This just leads to a back and forth
> that is harmfull to the team. 

It's pretty funny how you made a bunch of accusations against me in the
same mail here, and then you say to stop accusations.

There wasn't even an accusation in the commit message, but you somehow
interpreted it as direct against against you. And then you go here and
tell us to "calm down". Strange.

> If you think iam doing something bad or wrong, talk with me about it please
> theres private mail there is IRC. Hasnt it always been a misunderstanding of
> some form in the past?

Last time I wrote something to you privately on IRC you quoted it in
public on the mailing list. De-escalation, dude.

> I dont block patches generally ... If a patch is really bad many people will

You sometimes make quite unclear comments. Contributors don't know what
they're supposed to change in a patch or how. Others have occasionally
complained about this too.

In any case, I remember you aggressively blocked some things in the
past that required a few flames to get them unblocked.

On the other hand, I don't think I overly insistent in blocking patches
in general. When I end up in a flame, it's usually about me trying to
get my patches unblocked, not blocking other's work. (Remember those
damn side data merging changes? You blocked something until I made some
dumb change that got neutralized later anyway.)

I also remember how I wanted to delay the previous FFmpeg release to
get the hwaccel changes in (since mpv relies on them), but you
overruled me. Hilariously enough, now a new release is not happening
because of that shit show certain FFmpeg devs put on around those
component list patches. The contributor was literally begging for what
directions to take, but the project is unable to get anything done.
(Your role in this was to list the same arguments again and forgetting
or not finishing previous discussions about it.)

Last but not least I want to say your attempt to make a list of me
blocking things (see above) wasn't overly impressive.

So don't go around and claim I'm somehow holding things back. OK?

> object to it, it doesnt require me beyond maybe pointing to the issue to make
> sure others are aware of it ...
> 
> Peace
> 
> [...]

I probably will stop responding to such replies of yours. What the fuck
is this even?
James Almer March 31, 2018, 1:13 a.m.
On 3/30/2018 9:55 PM, wm4 wrote:
> On Sat, 31 Mar 2018 01:55:52 +0200
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
>> On Fri, Mar 30, 2018 at 01:31:35PM +0200, Paul B Mahol wrote:
>> [...]
>>> Why you and some other 'old' developers have urge to block every single patch
>>> that comes from some developers?  
>>
>> Thats a pretty serious accusation, you should not throw this around
>> lightly unless its true.
>>
>> Also i would generally prefer not to talk about people in relation to
>> who did what as its alienating and this is bad for us as a team.
>> But being accused of something i did not do, how could i defend myself
>> without refering to what was actually said and by whom. And these
>> accusations keep comming up ...
> 
> Maybe they're true at least to some extent? Have you ever thought about
> that. Others complained too. Could also list Libav as argument.
> 
> I won't claim I'm 100% innocent (just like I don't claim you're 100%
> evil). But there is definitely something wrong here with how you
> present things.
> 
>> Whos patches are being blocked ?
>> I assume by "some developers" you mean wm4 ? If so lets look at this
>> case
> 
> Oh, so you're singling me out as the evil one? That's of course a
> direct attack against me. Also since bystanders will trust your word
> more than mine (since you're universally known as "the" FFmpeg guy),
> this is pretty a pretty serious attack against me.
> 
> Anyway, my patches have been blocked or bikeshedded in the past as well
> by certain people (including yourself). Blocking/rejecting patches is
> part of the normal development process. (What matters is _how_ it
> happens and the social aspects around it.)
> 
>> wm4 blocked tobias av log patch yesterday (Re: [FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function)
>>     "I'd like to block it, because I don't see it as a good thing that more
>>      fftools specific stuff is leaking into the generic libs. Sorry."
> 
> I'd rather say: you blocked Tobias' initial patch, even though the patch
> was completely fine. I was not even aware you objected to me blocking
> the second version of the patch. You didn't reply to my arguments, or
> reply at all.
> 
>> and in the "[FFmpeg-devel] [RFC] avpriv cleanup" thread 3 days before 
>>     the debate went in circles until i gave up as it just got too much
>>     effort for the small gain of the change i wanted to do.
>>     Not technically blocked no
> 
> Well, I also spend a lot of time going in circles with you, instead of
> making progress. Like in this thread right now.
> 
> Can you explain how I "blocked" this? I explicitly encouraged you to
> post concrete proposals (though you never did that), and my only gripe
> with this was that the discussion was overly broad, with some danger
> that you'd just make all avpriv functions public or so.
> 
> I'm actually not sure what you're complaining about wrt. to that
> avpriv thread. This is how an overly broad discussion will go. So why
> did you start one. What was your desired or expected outcome of this
> thread? Everyone agrees and then does nothing, because there couldn't
> really be any actionable result?
> 
> Actually, my patch makes it unnecessary to make one of those avpriv_
> functions public, so you could say I contributed to this in a
> productive way.
> 
>> and in this thread here with the roles flipped around his comment was
>>     "I don't intend to make such strange changes. If you don't have anything
>>      actual to contribute, I will push this patch on monday."
>>      
>> He also made similar comments to nicolas. but lets stay with this here
> 
> Nicolas George? That guy who aggressively calls me a troll etc. and
> even refuses to discuss anything directly with me?
> 
>> The "strange changes" here are a request to split out renaming an identifer
>> that is required by our development policy, and everyone else splits their
>> renamings out too and i do not remember any other developer questioning that
>> such changes improve the git log readability
>>
>> you can check https://ffmpeg.org/developer.html
>> "Cosmetic changes should be kept in separate patches."
> 
> These changes are inherent, functional parts of the patch. It would
> make little sense to split them out. It'd just obfuscate the
> consequences of the change. I explained this, but you didn't really
> respond to it and just repeated your original request.
> 
> The policy is NOT to split changes in nonsense ways just to reduce
> commit sizes. Asking to split patches just for the hell of it is pretty
> strange.
> 
>> And which patch do you belive iam blocking ? I was not intending of blocking any
>> patch. 
> 
> You JUST complained that I wanted to push this patch as is.
> 
> So what is it:
> a) my patch is not blocked, you're content with my reply that I don't
>    want to split out those changes we talked about to a separate commit
> b) my patch is blocked until I send a new split version of the patch
> c) something else which you haven't said yet or something equally vague
>    that you intend to complain about after I push the patch
> 
> I suspect it's actually c).
> 
>> Its really odd, if i look at just the last few days, who blocked patches
>> and who got accused of it. It just makes no sense. There is not much overlap
>> between who blocked patches and who got accused of blocking patches.
>>
>> Either way these hostilities we have here are not good, please calm down, iam
>> not blocking any patches. If some other developer, wm4, nicolas or whoever 
>> does, iam sure he has a reason for it, even if someone disagrees. 
>> The best for such a case would be
>> to calmly and respectfully discuss. Escalation will likely not achive anything
>> except Escalation.
>>
>> Also lets stop accusing people, of things. This just leads to a back and forth
>> that is harmfull to the team. 
> 
> It's pretty funny how you made a bunch of accusations against me in the
> same mail here, and then you say to stop accusations.
> 
> There wasn't even an accusation in the commit message, but you somehow
> interpreted it as direct against against you. And then you go here and
> tell us to "calm down". Strange.
> 
>> If you think iam doing something bad or wrong, talk with me about it please
>> theres private mail there is IRC. Hasnt it always been a misunderstanding of
>> some form in the past?
> 
> Last time I wrote something to you privately on IRC you quoted it in
> public on the mailing list. De-escalation, dude.
> 
>> I dont block patches generally ... If a patch is really bad many people will
> 
> You sometimes make quite unclear comments. Contributors don't know what
> they're supposed to change in a patch or how. Others have occasionally
> complained about this too.
> 
> In any case, I remember you aggressively blocked some things in the
> past that required a few flames to get them unblocked.
> 
> On the other hand, I don't think I overly insistent in blocking patches
> in general. When I end up in a flame, it's usually about me trying to
> get my patches unblocked, not blocking other's work. (Remember those
> damn side data merging changes? You blocked something until I made some
> dumb change that got neutralized later anyway.)
> 
> I also remember how I wanted to delay the previous FFmpeg release to
> get the hwaccel changes in (since mpv relies on them), but you
> overruled me. Hilariously enough, now a new release is not happening
> because of that shit show certain FFmpeg devs put on around those
> component list patches. The contributor was literally begging for what
> directions to take, but the project is unable to get anything done.
> (Your role in this was to list the same arguments again and forgetting
> or not finishing previous discussions about it.)
> 
> Last but not least I want to say your attempt to make a list of me
> blocking things (see above) wasn't overly impressive.
> 
> So don't go around and claim I'm somehow holding things back. OK?
> 
>> object to it, it doesnt require me beyond maybe pointing to the issue to make
>> sure others are aware of it ...
>>
>> Peace
>>
>> [...]
> 
> I probably will stop responding to such replies of yours. What the fuck
> is this even?

Can we stop having every other thread full of accusations, attacks and
all kinds of crap? It's getting out of hand, and it's always when you
two are involved, but never when any one of you talks with other devs.

I don't know who is to blame, but there's definitely some predisposition
whenever there's an exchange between the two of you to elevate every
damn thing, no matter how small, into a fucking fight.

Stop thinking the other is out there only to mess with you.
wm4 April 2, 2018, 1:54 p.m.
On Thu, 29 Mar 2018 15:30:43 +0200
wm4 <nfxjfg@googlemail.com> wrote:

> PSEUDOPAL pixel formats are not paletted, but carried a palette with the

If nobody has any comments, I will push tonight or tomorrow or so.

Patch hide | download patch | download mbox

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index a4ac6972a2..6965b9f679 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -3118,7 +3118,9 @@  static void ffprobe_show_pixel_formats(WriterContext *w)
             PRINT_PIX_FMT_FLAG(HWACCEL,   "hwaccel");
             PRINT_PIX_FMT_FLAG(PLANAR,    "planar");
             PRINT_PIX_FMT_FLAG(RGB,       "rgb");
+#if FF_API_PSEUDOPAL
             PRINT_PIX_FMT_FLAG(PSEUDOPAL, "pseudopal");
+#endif
             PRINT_PIX_FMT_FLAG(ALPHA,     "alpha");
             writer_print_section_footer(w);
         }
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 40c8a8855c..d883a5f9fc 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1614,7 +1614,7 @@  static int video_get_buffer(AVCodecContext *s, AVFrame *pic)
         pic->linesize[i] = 0;
     }
     if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
-        desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)
+        ((desc->flags & FF_PSEUDOPAL) && pic->data[1]))
         avpriv_set_systematic_pal2((uint32_t *)pic->data[1], pic->format);
 
     if (s->debug & FF_DEBUG_BUFFERS)
@@ -1782,9 +1782,6 @@  static void validate_avframe_allocation(AVCodecContext *avctx, AVFrame *frame)
         for (i = 0; i < num_planes; i++) {
             av_assert0(frame->data[i]);
         }
-        // For now do not enforce anything for palette of pseudopal formats
-        if (num_planes == 1 && (flags & AV_PIX_FMT_FLAG_PSEUDOPAL))
-            num_planes = 2;
         // For formats without data like hwaccel allow unused pointers to be non-NULL.
         for (i = num_planes; num_planes > 0 && i < FF_ARRAY_ELEMS(frame->data); i++) {
             if (frame->data[i])
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index b4a183c5b7..7658a51685 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -950,7 +950,7 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
 
             }
             if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
-                desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) {
+                desc->flags & FF_PSEUDOPAL) {
                 dst[1] = p->data[1];
                 src[1] = f->last_picture.f->data[1];
             }
diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
index 1893b26444..53f5b76e93 100644
--- a/libavcodec/rawdec.c
+++ b/libavcodec/rawdec.c
@@ -92,12 +92,14 @@  static av_cold int raw_init_decoder(AVCodecContext *avctx)
         return AVERROR(EINVAL);
     }
 
-    if (desc->flags & (AV_PIX_FMT_FLAG_PAL | AV_PIX_FMT_FLAG_PSEUDOPAL)) {
+    if (desc->flags & (AV_PIX_FMT_FLAG_PAL | FF_PSEUDOPAL)) {
         context->palette = av_buffer_alloc(AVPALETTE_SIZE);
         if (!context->palette)
             return AVERROR(ENOMEM);
+#if FF_API_PSEUDOPAL
         if (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)
             avpriv_set_systematic_pal2((uint32_t*)context->palette->data, avctx->pix_fmt);
+#endif
         else {
             memset(context->palette->data, 0, AVPALETTE_SIZE);
             if (avctx->bits_per_coded_sample == 1)
@@ -423,7 +425,7 @@  static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
     }
 
     if ((avctx->pix_fmt == AV_PIX_FMT_PAL8 && buf_size < context->frame_size) ||
-        (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)) {
+        (desc->flags & FF_PSEUDOPAL)) {
         frame->buf[1]  = av_buffer_ref(context->palette);
         if (!frame->buf[1]) {
             av_buffer_unref(&frame->buf[0]);
diff --git a/libavcodec/smvjpegdec.c b/libavcodec/smvjpegdec.c
index 0b05d19f7b..7ea82ebfee 100644
--- a/libavcodec/smvjpegdec.c
+++ b/libavcodec/smvjpegdec.c
@@ -71,7 +71,7 @@  static inline void smv_img_pnt(uint8_t *dst_data[4], uint8_t *src_data[4],
             src_linesizes[i], h, nlines);
     }
     if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
-        desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)
+        desc->flags & FF_PSEUDOPAL)
         dst_data[1] = src_data[1];
 }
 
diff --git a/libavfilter/drawutils.c b/libavfilter/drawutils.c
index 17e26c764a..db8c7a6226 100644
--- a/libavfilter/drawutils.c
+++ b/libavfilter/drawutils.c
@@ -184,7 +184,7 @@  int ff_draw_init(FFDrawContext *draw, enum AVPixelFormat format, unsigned flags)
 
     if (!desc || !desc->name)
         return AVERROR(EINVAL);
-    if (desc->flags & ~(AV_PIX_FMT_FLAG_PLANAR | AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PSEUDOPAL | AV_PIX_FMT_FLAG_ALPHA))
+    if (desc->flags & ~(AV_PIX_FMT_FLAG_PLANAR | AV_PIX_FMT_FLAG_RGB | FF_PSEUDOPAL | AV_PIX_FMT_FLAG_ALPHA))
         return AVERROR(ENOSYS);
     if (format == AV_PIX_FMT_P010LE || format == AV_PIX_FMT_P010BE || format == AV_PIX_FMT_P016LE || format == AV_PIX_FMT_P016BE)
         return AVERROR(ENOSYS);
diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c
index da2ac5cf69..3b178cebb8 100644
--- a/libavfilter/framepool.c
+++ b/libavfilter/framepool.c
@@ -103,7 +103,7 @@  FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(int size),
     }
 
     if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
-        desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) {
+        desc->flags & FF_PSEUDOPAL) {
         pool->pools[1] = av_buffer_pool_init(AVPALETTE_SIZE, alloc);
         if (!pool->pools[1])
             goto fail;
@@ -227,7 +227,7 @@  AVFrame *ff_frame_pool_get(FFFramePool *pool)
         }
 
         if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
-            desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) {
+            desc->flags & FF_PSEUDOPAL) {
             enum AVPixelFormat format =
                 pool->format == AV_PIX_FMT_PAL8 ? AV_PIX_FMT_BGR8 : pool->format;
 
diff --git a/libavfilter/vf_crop.c b/libavfilter/vf_crop.c
index 0fdc4949e3..84be4c7d0d 100644
--- a/libavfilter/vf_crop.c
+++ b/libavfilter/vf_crop.c
@@ -288,7 +288,7 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     frame->data[0] += s->y * frame->linesize[0];
     frame->data[0] += s->x * s->max_step[0];
 
-    if (!(desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)) {
+    if (!(desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL)) {
         for (i = 1; i < 3; i ++) {
             if (frame->data[i]) {
                 frame->data[i] += (s->y >> s->vsub) * frame->linesize[i];
diff --git a/libavfilter/vf_pixdesctest.c b/libavfilter/vf_pixdesctest.c
index d6423acb91..2d0749e20b 100644
--- a/libavfilter/vf_pixdesctest.c
+++ b/libavfilter/vf_pixdesctest.c
@@ -81,7 +81,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 
     /* copy palette */
     if (priv->pix_desc->flags & AV_PIX_FMT_FLAG_PAL ||
-        priv->pix_desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)
+        ((priv->pix_desc->flags & FF_PSEUDOPAL) && out->data[1] && in->data[1]))
         memcpy(out->data[1], in->data[1], AVPALETTE_SIZE);
 
     for (c = 0; c < priv->pix_desc->nb_components; c++) {
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 9f45032e85..f741419e7e 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -261,11 +261,10 @@  static int config_props(AVFilterLink *outlink)
 
     /* TODO: make algorithm configurable */
 
-    scale->input_is_pal = desc->flags & AV_PIX_FMT_FLAG_PAL ||
-                          desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL;
+    scale->input_is_pal = desc->flags & AV_PIX_FMT_FLAG_PAL;
     if (outfmt == AV_PIX_FMT_PAL8) outfmt = AV_PIX_FMT_BGR8;
     scale->output_is_pal = av_pix_fmt_desc_get(outfmt)->flags & AV_PIX_FMT_FLAG_PAL ||
-                           av_pix_fmt_desc_get(outfmt)->flags & AV_PIX_FMT_FLAG_PSEUDOPAL;
+                           av_pix_fmt_desc_get(outfmt)->flags & FF_PSEUDOPAL;
 
     if (scale->sws)
         sws_freeContext(scale->sws);
diff --git a/libavfilter/vf_shuffleplanes.c b/libavfilter/vf_shuffleplanes.c
index 4bc7b79f87..32d2d585f3 100644
--- a/libavfilter/vf_shuffleplanes.c
+++ b/libavfilter/vf_shuffleplanes.c
@@ -69,7 +69,7 @@  static av_cold int shuffleplanes_config_input(AVFilterLink *inlink)
         }
 
         if ((desc->flags & AV_PIX_FMT_FLAG_PAL ||
-             desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) &&
+             desc->flags & FF_PSEUDOPAL) &&
             (i == 1) != (s->map[i] == 1)) {
             av_log(ctx, AV_LOG_ERROR,
                    "Cannot map between a palette plane and a data plane.\n");
diff --git a/libavutil/frame.c b/libavutil/frame.c
index ea13cd3ed6..00215ac29a 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -247,7 +247,7 @@  static int get_video_buffer(AVFrame *frame, int align)
 
         frame->data[i] = frame->buf[i]->data;
     }
-    if (desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) {
+    if (desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL) {
         av_buffer_unref(&frame->buf[1]);
         frame->buf[1] = av_buffer_alloc(AVPALETTE_SIZE);
         if (!frame->buf[1])
@@ -848,7 +848,7 @@  static int calc_cropping_offsets(size_t offsets[4], const AVFrame *frame,
         int shift_x = (i == 1 || i == 2) ? desc->log2_chroma_w : 0;
         int shift_y = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
 
-        if (desc->flags & (AV_PIX_FMT_FLAG_PAL | AV_PIX_FMT_FLAG_PSEUDOPAL) && i == 1) {
+        if (desc->flags & (AV_PIX_FMT_FLAG_PAL | FF_PSEUDOPAL) && i == 1) {
             offsets[i] = 0;
             break;
         }
diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index 5af4fc20a0..4938a7ef67 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -125,7 +125,7 @@  int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
     size[0] = linesizes[0] * height;
 
     if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
-        desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) {
+        desc->flags & FF_PSEUDOPAL) {
         data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits words */
         return size[0] + 256 * 4;
     }
@@ -216,7 +216,7 @@  int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
         av_free(buf);
         return ret;
     }
-    if (desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) {
+    if (desc->flags & AV_PIX_FMT_FLAG_PAL || (desc->flags & FF_PSEUDOPAL && pointers[1])) {
         avpriv_set_systematic_pal2((uint32_t*)pointers[1], pix_fmt);
         if (align < 4) {
             av_log(NULL, AV_LOG_ERROR, "Formats with a palette require a minimum alignment of 4\n");
@@ -225,7 +225,7 @@  int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
     }
 
     if ((desc->flags & AV_PIX_FMT_FLAG_PAL ||
-         desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) &&
+         desc->flags & FF_PSEUDOPAL) && pointers[1] &&
         pointers[1] - pointers[0] > linesizes[0] * h) {
         /* zero-initialize the padding before the palette */
         memset(pointers[0] + linesizes[0] * h, 0,
@@ -354,12 +354,13 @@  static void image_copy(uint8_t *dst_data[4], const ptrdiff_t dst_linesizes[4],
         return;
 
     if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
-        desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) {
+        desc->flags & FF_PSEUDOPAL) {
         copy_plane(dst_data[0], dst_linesizes[0],
                    src_data[0], src_linesizes[0],
                    width, height);
         /* copy the palette */
-        memcpy(dst_data[1], src_data[1], 4*256);
+        if ((desc->flags & AV_PIX_FMT_FLAG_PAL) || (dst_data[1] && src_data[1]))
+            memcpy(dst_data[1], src_data[1], 4*256);
     } else {
         int i, planes_nb = 0;
 
@@ -442,7 +443,7 @@  int av_image_get_buffer_size(enum AVPixelFormat pix_fmt,
         return ret;
 
     // do not include palette for these pseudo-paletted formats
-    if (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)
+    if (desc->flags & FF_PSEUDOPAL)
         return FFALIGN(width, align) * height;
 
     return av_image_fill_arrays(data, linesize, NULL, pix_fmt,
diff --git a/libavutil/internal.h b/libavutil/internal.h
index c77dfa7d3c..06bd561e82 100644
--- a/libavutil/internal.h
+++ b/libavutil/internal.h
@@ -360,4 +360,13 @@  void ff_check_pixfmt_descriptors(void);
  */
 int avpriv_dict_set_timestamp(AVDictionary **dict, const char *key, int64_t timestamp);
 
+// Helper macro for AV_PIX_FMT_FLAG_PSEUDOPAL deprecation. Code inside FFmpeg
+// should always use FF_PSEUDOPAL. Once the public API flag gets removed, all
+// code using it is dead code.
+#if FF_API_PSEUDOPAL
+#define FF_PSEUDOPAL AV_PIX_FMT_FLAG_PSEUDOPAL
+#else
+#define FF_PSEUDOPAL 0
+#endif
+
 #endif /* AVUTIL_INTERNAL_H */
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index e77d5f44b9..8ed52751c1 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -257,7 +257,7 @@  static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
         .comp = {
             { 0, 1, 0, 0, 8, 0, 7, 1 },        /* Y */
         },
-        .flags = AV_PIX_FMT_FLAG_PSEUDOPAL,
+        .flags = FF_PSEUDOPAL,
         .alias = "gray8,y8",
     },
     [AV_PIX_FMT_MONOWHITE] = {
@@ -362,7 +362,7 @@  static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
             { 0, 1, 0, 3, 3, 0, 2, 1 },        /* G */
             { 0, 1, 0, 6, 2, 0, 1, 1 },        /* B */
         },
-        .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PSEUDOPAL,
+        .flags = AV_PIX_FMT_FLAG_RGB | FF_PSEUDOPAL,
     },
     [AV_PIX_FMT_BGR4] = {
         .name = "bgr4",
@@ -386,7 +386,7 @@  static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
             { 0, 1, 0, 1, 2, 0, 1, 1 },        /* G */
             { 0, 1, 0, 3, 1, 0, 0, 1 },        /* B */
         },
-        .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PSEUDOPAL,
+        .flags = AV_PIX_FMT_FLAG_RGB | FF_PSEUDOPAL,
     },
     [AV_PIX_FMT_RGB8] = {
         .name = "rgb8",
@@ -398,7 +398,7 @@  static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
             { 0, 1, 0, 3, 3, 0, 2, 1 },        /* G */
             { 0, 1, 0, 0, 3, 0, 2, 1 },        /* B */
         },
-        .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PSEUDOPAL,
+        .flags = AV_PIX_FMT_FLAG_RGB | FF_PSEUDOPAL,
     },
     [AV_PIX_FMT_RGB4] = {
         .name = "rgb4",
@@ -422,7 +422,7 @@  static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
             { 0, 1, 0, 1, 2, 0, 1, 1 },        /* G */
             { 0, 1, 0, 0, 1, 0, 0, 1 },        /* B */
         },
-        .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PSEUDOPAL,
+        .flags = AV_PIX_FMT_FLAG_RGB | FF_PSEUDOPAL,
     },
     [AV_PIX_FMT_NV12] = {
         .name = "nv12",
diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
index ea046033a3..feb3eba5e0 100644
--- a/libavutil/pixdesc.h
+++ b/libavutil/pixdesc.h
@@ -154,6 +154,14 @@  typedef struct AVPixFmtDescriptor {
  * in some cases be simpler. Or the data can be interpreted purely based on
  * the pixel format without using the palette.
  * An example of a pseudo-paletted format is AV_PIX_FMT_GRAY8
+ *
+ * @deprecated This flag is deprecated, and will be removed. When it is removed,
+ * the extra palette allocation in AVFrame.data[1] is removed as well. Only
+ * actual paletted formats (as indicated by AV_PIX_FMT_FLAG_PAL) will have a
+ * palette. Starting with FFmpeg versions which have this flag deprecated, the
+ * extra "pseudo" palette is already ignored, and API users are not required to
+ * allocate a palette for AV_PIX_FMT_FLAG_PSEUDOPAL formats (it was require
+ * before the deprecation, though).
  */
 #define AV_PIX_FMT_FLAG_PSEUDOPAL    (1 << 6)
 
diff --git a/libavutil/version.h b/libavutil/version.h
index d3dd2df544..491b841403 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -126,6 +126,9 @@ 
 #ifndef FF_API_FRAME_GET_SET
 #define FF_API_FRAME_GET_SET            (LIBAVUTIL_VERSION_MAJOR < 57)
 #endif
+#ifndef FF_API_PSEUDOPAL
+#define FF_API_PSEUDOPAL                (LIBAVUTIL_VERSION_MAJOR < 57)
+#endif
 
 
 /**
diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index c9120d8f5f..1703856ab2 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -806,9 +806,17 @@  static av_always_inline int isPlanarRGB(enum AVPixelFormat pix_fmt)
 
 static av_always_inline int usePal(enum AVPixelFormat pix_fmt)
 {
-    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
-    av_assert0(desc);
-    return (desc->flags & AV_PIX_FMT_FLAG_PAL) || (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL);
+    switch (pix_fmt) {
+    case AV_PIX_FMT_PAL8:
+    case AV_PIX_FMT_BGR4_BYTE:
+    case AV_PIX_FMT_BGR8:
+    case AV_PIX_FMT_GRAY8:
+    case AV_PIX_FMT_RGB4_BYTE:
+    case AV_PIX_FMT_RGB8:
+        return 1;
+    default:
+        return 0;
+    }
 }
 
 extern const uint64_t ff_dither4[2];