diff mbox series

[FFmpeg-devel,3/5] avfilter/drawutils: simplify xyz format check

Message ID 20231026121901.68926-3-ffmpeg@haasn.xyz
State New
Headers show
Series [FFmpeg-devel,1/5] avfilter/drawutils: ban XYZ formats | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Niklas Haas Oct. 26, 2023, 12:18 p.m. UTC
From: Niklas Haas <git@haasn.dev>

---
 libavfilter/drawutils.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Nicolas George Oct. 26, 2023, 12:23 p.m. UTC | #1
Niklas Haas (12023-10-26):
> From: Niklas Haas <git@haasn.dev>
> 
> ---
>  libavfilter/drawutils.c | 3 ---
>  1 file changed, 3 deletions(-)

Please merge this with patch 1.

Regards,
James Almer Oct. 26, 2023, 1:13 p.m. UTC | #2
On 10/26/2023 9:23 AM, Nicolas George wrote:
> Niklas Haas (12023-10-26):
>> From: Niklas Haas <git@haasn.dev>
>>
>> ---
>>   libavfilter/drawutils.c | 3 ---
>>   1 file changed, 3 deletions(-)
> 
> Please merge this with patch 1.

This patch reverts patch 1, so merging is basically just dropping both.
I assume the reason he did things this way is to prevent patch 2 (an 
avutil patch adding new API) from changing the output filter-pixfmts-pad.
Nicolas George Oct. 26, 2023, 1:30 p.m. UTC | #3
James Almer (12023-10-26):
> This patch reverts patch 1, so merging is basically just dropping both.
> I assume the reason he did things this way is to prevent patch 2 (an avutil
> patch adding new API) from changing the output filter-pixfmts-pad.

Oh, I read the commit message while your read the actual patch.

Niklas: patch 3 is incorrect, please fix it and drop patch 1.

Regards,
Niklas Haas Oct. 26, 2023, 2:05 p.m. UTC | #4
On Thu, 26 Oct 2023 10:13:03 -0300 James Almer <jamrial@gmail.com> wrote:
> On 10/26/2023 9:23 AM, Nicolas George wrote:
> > Niklas Haas (12023-10-26):
> >> From: Niklas Haas <git@haasn.dev>
> >>
> >> ---
> >>   libavfilter/drawutils.c | 3 ---
> >>   1 file changed, 3 deletions(-)
> > 
> > Please merge this with patch 1.
> 
> This patch reverts patch 1, so merging is basically just dropping both.
> I assume the reason he did things this way is to prevent patch 2 (an 
> avutil patch adding new API) from changing the output filter-pixfmts-pad.

Yes, exactly. But if you want, I can merge the FATE ref change into
patch 2. I just thought this way separates the logic change (bugfix in
drawutils) from the new API addition, which is IMO cleaner.
Marvin Scholz Oct. 26, 2023, 2:13 p.m. UTC | #5
On 26 Oct 2023, at 16:05, Niklas Haas wrote:

> On Thu, 26 Oct 2023 10:13:03 -0300 James Almer <jamrial@gmail.com> wrote:
>> On 10/26/2023 9:23 AM, Nicolas George wrote:
>>> Niklas Haas (12023-10-26):
>>>> From: Niklas Haas <git@haasn.dev>
>>>>
>>>> ---
>>>>   libavfilter/drawutils.c | 3 ---
>>>>   1 file changed, 3 deletions(-)
>>>
>>> Please merge this with patch 1.
>>
>> This patch reverts patch 1, so merging is basically just dropping both.
>> I assume the reason he did things this way is to prevent patch 2 (an
>> avutil patch adding new API) from changing the output filter-pixfmts-pad.
>
> Yes, exactly. But if you want, I can merge the FATE ref change into
> patch 2. I just thought this way separates the logic change (bugfix in
> drawutils) from the new API addition, which is IMO cleaner.

IMO it’s easier to understand with the patches separated.
Maybe clarify the commit message a bit though about this being
replaced by the new flag in a follow-up commit?

Also I am confused about this patch (simplify xyz format check),
as the message says „simplify“ but you just remove it? Is it covered
already by an existing check in this file not touched by the patch?
Then the message should probably be „remove redundant check“ and
clarify why it is redundant now?

> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavfilter/drawutils.c b/libavfilter/drawutils.c
index c31ab6bd5a..1081938d86 100644
--- a/libavfilter/drawutils.c
+++ b/libavfilter/drawutils.c
@@ -22,7 +22,6 @@ 
 #include <string.h>
 
 #include "libavutil/avassert.h"
-#include "libavutil/avstring.h"
 #include "libavutil/avutil.h"
 #include "libavutil/csp.h"
 #include "libavutil/intreadwrite.h"
@@ -94,8 +93,6 @@  int ff_draw_init2(FFDrawContext *draw, enum AVPixelFormat format, enum AVColorSp
         return AVERROR(ENOSYS);
     if (desc->flags & ~(AV_PIX_FMT_FLAG_PLANAR | AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_ALPHA))
         return AVERROR(ENOSYS);
-    if (av_strstart(desc->name, "xyz", NULL))
-        return AVERROR(ENOSYS);
     if (csp == AVCOL_SPC_UNSPECIFIED)
         csp = (desc->flags & AV_PIX_FMT_FLAG_RGB) ? AVCOL_SPC_RGB : AVCOL_SPC_SMPTE170M;
     if (!(desc->flags & AV_PIX_FMT_FLAG_RGB) && !(luma = av_csp_luma_coeffs_from_avcsp(csp)))