diff mbox series

[FFmpeg-devel] lavf/nut: Explicitely add tags for NV12 and NV21

Message ID CAB0OVGrZnwi31bgQpP=rjTQM4BW0YifnJ0hW3WoWJ02Wk-KFDA@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] lavf/nut: Explicitely add tags for NV12 and NV21 | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Carl Eugen Hoyos Jan. 29, 2020, 12:44 a.m. UTC 
Hi!

Mentioned tags are already used in nut by fate.

Please comment, Carl Eugen

Comments

Michael Niedermayer Jan. 29, 2020, 1:33 p.m. UTC | #1 
On Wed, Jan 29, 2020 at 01:44:52AM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> Mentioned tags are already used in nut by fate.
> 
> Please comment, Carl Eugen

>  nut.c |    3 +++
>  1 file changed, 3 insertions(+)
> ae60da7cb36c94b7cab3bd17f02bd9403c8d2ff8  0001-lavf-nut-Explicitely-add-NV12-NV21-tags.patch
> From dd52b20ce2eea008a0c58c09f768e5ef92133578 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Wed, 29 Jan 2020 01:43:24 +0100
> Subject: [PATCH] lavf/nut: Explicitely add NV12/NV21 tags.
> 
> These are already used by fate.
> ---
>  libavformat/nut.c | 3 +++
>  1 file changed, 3 insertions(+)

they should be added to nut4cc.txt first

https://lists.mplayerhq.hu/mailman/listinfo/nut-devel

after that, LGTM

thx

[...]
Paul B Mahol Jan. 29, 2020, 1:44 p.m. UTC | #2 
On 1/29/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Wed, Jan 29, 2020 at 01:44:52AM +0100, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Mentioned tags are already used in nut by fate.
>>
>> Please comment, Carl Eugen
>
>>  nut.c |    3 +++
>>  1 file changed, 3 insertions(+)
>> ae60da7cb36c94b7cab3bd17f02bd9403c8d2ff8
>> 0001-lavf-nut-Explicitely-add-NV12-NV21-tags.patch
>> From dd52b20ce2eea008a0c58c09f768e5ef92133578 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Wed, 29 Jan 2020 01:43:24 +0100
>> Subject: [PATCH] lavf/nut: Explicitely add NV12/NV21 tags.
>>
>> These are already used by fate.
>> ---
>>  libavformat/nut.c | 3 +++
>>  1 file changed, 3 insertions(+)
>
> they should be added to nut4cc.txt first
>
> https://lists.mplayerhq.hu/mailman/listinfo/nut-devel
>
> after that, LGTM

No, patch is LGTM, push at will.

Mentioned list should be removed.

>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many things microsoft did are stupid, but not doing something just because
> microsoft did it is even more stupid. If everything ms did were stupid they
> would be bankrupt already.
>
Michael Niedermayer Jan. 30, 2020, 1 p.m. UTC | #3 
On Wed, Jan 29, 2020 at 02:44:02PM +0100, Paul B Mahol wrote:
> On 1/29/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Wed, Jan 29, 2020 at 01:44:52AM +0100, Carl Eugen Hoyos wrote:
> >> Hi!
> >>
> >> Mentioned tags are already used in nut by fate.
> >>
> >> Please comment, Carl Eugen
> >
> >>  nut.c |    3 +++
> >>  1 file changed, 3 insertions(+)
> >> ae60da7cb36c94b7cab3bd17f02bd9403c8d2ff8
> >> 0001-lavf-nut-Explicitely-add-NV12-NV21-tags.patch
> >> From dd52b20ce2eea008a0c58c09f768e5ef92133578 Mon Sep 17 00:00:00 2001
> >> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> >> Date: Wed, 29 Jan 2020 01:43:24 +0100
> >> Subject: [PATCH] lavf/nut: Explicitely add NV12/NV21 tags.
> >>
> >> These are already used by fate.
> >> ---
> >>  libavformat/nut.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >
> > they should be added to nut4cc.txt first
> >
> > https://lists.mplayerhq.hu/mailman/listinfo/nut-devel
> >
> > after that, LGTM
> 
> No, patch is LGTM, push at will.
> 
> Mentioned list should be removed.

nut4cc.txt lists the 4ccs for nut, nut also uses the AVI 4ccs

https://git.ffmpeg.org/gitweb/nut.git/blob/HEAD:/docs/nut4cc.txt
It does not list NV12 or NV21
avi does list them
https://www.fourcc.org/pixel-format/yuv-nv12/

The patch starting this thread adds the NV12/21 fourccs to the nut specific
list. That is only correct if they are in the nut specific fourcc list
of the nut specification (nut4cc.txt).
Thats why i suggest to add them there first or maybe we do not need to
add them to the nut specific list in the implementation at all and its
good enough that they are in the avi list.

But the combination of the implementation listing a fourcc in its nut
specific list while the nut specification not listing it does not seem
correct to me.

Thanks

[...]
Paul B Mahol Jan. 30, 2020, 1:04 p.m. UTC | #4 
On 1/30/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Wed, Jan 29, 2020 at 02:44:02PM +0100, Paul B Mahol wrote:
>> On 1/29/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> > On Wed, Jan 29, 2020 at 01:44:52AM +0100, Carl Eugen Hoyos wrote:
>> >> Hi!
>> >>
>> >> Mentioned tags are already used in nut by fate.
>> >>
>> >> Please comment, Carl Eugen
>> >
>> >>  nut.c |    3 +++
>> >>  1 file changed, 3 insertions(+)
>> >> ae60da7cb36c94b7cab3bd17f02bd9403c8d2ff8
>> >> 0001-lavf-nut-Explicitely-add-NV12-NV21-tags.patch
>> >> From dd52b20ce2eea008a0c58c09f768e5ef92133578 Mon Sep 17 00:00:00 2001
>> >> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> >> Date: Wed, 29 Jan 2020 01:43:24 +0100
>> >> Subject: [PATCH] lavf/nut: Explicitely add NV12/NV21 tags.
>> >>
>> >> These are already used by fate.
>> >> ---
>> >>  libavformat/nut.c | 3 +++
>> >>  1 file changed, 3 insertions(+)
>> >
>> > they should be added to nut4cc.txt first
>> >
>> > https://lists.mplayerhq.hu/mailman/listinfo/nut-devel
>> >
>> > after that, LGTM
>>
>> No, patch is LGTM, push at will.
>>
>> Mentioned list should be removed.
>
> nut4cc.txt lists the 4ccs for nut, nut also uses the AVI 4ccs
>
> https://git.ffmpeg.org/gitweb/nut.git/blob/HEAD:/docs/nut4cc.txt
> It does not list NV12 or NV21
> avi does list them
> https://www.fourcc.org/pixel-format/yuv-nv12/
>
> The patch starting this thread adds the NV12/21 fourccs to the nut specific
> list. That is only correct if they are in the nut specific fourcc list
> of the nut specification (nut4cc.txt).
> Thats why i suggest to add them there first or maybe we do not need to
> add them to the nut specific list in the implementation at all and its
> good enough that they are in the avi list.
>
> But the combination of the implementation listing a fourcc in its nut
> specific list while the nut specification not listing it does not seem
> correct to me.
>

Than riff.c is petter place for this. And is already there.
Why they are not used?

> Thanks
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Avoid a single point of failure, be that a person or equipment.
>
diff mbox series

Patch

From dd52b20ce2eea008a0c58c09f768e5ef92133578 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Wed, 29 Jan 2020 01:43:24 +0100
Subject: [PATCH] lavf/nut: Explicitely add NV12/NV21 tags.

These are already used by fate.
---
 libavformat/nut.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavformat/nut.c b/libavformat/nut.c
index d6993239a3..fb7d7b512d 100644
--- a/libavformat/nut.c
+++ b/libavformat/nut.c
@@ -163,6 +163,9 @@  const AVCodecTag ff_nut_video_tags[] = {
     { AV_CODEC_ID_RAWVIDEO,         MKTAG('Y', '1',   0,  14) },
     { AV_CODEC_ID_RAWVIDEO,         MKTAG(14,    0, '1', 'Y') },
 
+    { AV_CODEC_ID_RAWVIDEO,         MKTAG('N', 'V', '1', '2') },
+    { AV_CODEC_ID_RAWVIDEO,         MKTAG('N', 'V', '2', '1') },
+
     { AV_CODEC_ID_RAWVIDEO,         MKTAG('G', '3',   0,   8) },
 
     { AV_CODEC_ID_RAWVIDEO,         MKTAG('G', '3',   0,   9) },
-- 
2.24.1