diff mbox

[FFmpeg-devel] lavc/exr: Ignore long names flag

Message ID CAB0OVGrLS4BSu5GoMwPFbT_W0_FLMmbYsLbocKRwDnGdmqXDWQ@mail.gmail.com
State Superseded
Headers show

Commit Message

Carl Eugen Hoyos Jan. 30, 2018, 9:27 a.m. UTC
Hi!

Attached patch fixes ticket #6994, unknown tag names are ignored by FFmpeg.

Please comment, Carl Eugen

Comments

Paul B Mahol Jan. 30, 2018, 9:30 a.m. UTC | #1
On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> Hi!
>
> Attached patch fixes ticket #6994, unknown tag names are ignored by FFmpeg.
>
> Please comment, Carl Eugen
>

Really? Are you sure this does not cause not decoding previously
decodable files?
Carl Eugen Hoyos Jan. 30, 2018, 9:33 a.m. UTC | #2
2018-01-30 10:30 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>
>> Attached patch fixes ticket #6994, unknown tag names are
>> ignored by FFmpeg.
>
> Really? Are you sure this does not cause not decoding previously
> decodable files?

Could you point me to a sample?

Which previously working flags would be broken now?

Thank you, Carl Eugen
Paul B Mahol Jan. 30, 2018, 12:16 p.m. UTC | #3
On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2018-01-30 10:30 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>> On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>
>>> Attached patch fixes ticket #6994, unknown tag names are
>>> ignored by FFmpeg.
>>
>> Really? Are you sure this does not cause not decoding previously
>> decodable files?
>
> Could you point me to a sample?
>
> Which previously working flags would be broken now?

I'm afraid I can not share such files.
Carl Eugen Hoyos Jan. 30, 2018, 1:40 p.m. UTC | #4
2018-01-30 13:16 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> 2018-01-30 10:30 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>> On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>>
>>>> Attached patch fixes ticket #6994, unknown tag names are
>>>> ignored by FFmpeg.
>>>
>>> Really? Are you sure this does not cause not decoding previously
>>> decodable files?
>>
>> Could you point me to a sample?
>>
>> Which previously working flags would be broken now?
>
> I'm afraid I can not share such files.

Which flags are set in these files?
How can I produce such a file?

Carl Eugen
Martin Vignali Feb. 6, 2018, 5:59 p.m. UTC | #5
2018-01-30 14:40 GMT+01:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:

> 2018-01-30 13:16 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> > On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >> 2018-01-30 10:30 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> >>> On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >>>>
> >>>> Attached patch fixes ticket #6994, unknown tag names are
> >>>> ignored by FFmpeg.
> >>>
> >>> Really? Are you sure this does not cause not decoding previously
> >>> decodable files?
> >>
> >> Could you point me to a sample?
> >>
> >> Which previously working flags would be broken now?
> >
> > I'm afraid I can not share such files.
>
> Which flags are set in these files?
> How can I produce such a file?
>
>
>
Hello,

All the current flags of openexr can be found here (page 6) :
https://github.com/openexr/openexr/blob/develop/OpenEXR/doc/OpenEXRFileLayout.pdf

Maybe instead of log the flag value, we can be more explicit, about the
unsupported flag

Martin
Carl Eugen Hoyos Feb. 6, 2018, 11:30 p.m. UTC | #6
2018-01-30 14:40 GMT+01:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2018-01-30 13:16 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>> On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> 2018-01-30 10:30 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>>> On 1/30/18, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>>>
>>>>> Attached patch fixes ticket #6994, unknown tag names are
>>>>> ignored by FFmpeg.
>>>>
>>>> Really? Are you sure this does not cause not decoding previously
>>>> decodable files?
>>>
>>> Could you point me to a sample?
>>>
>>> Which previously working flags would be broken now?
>>
>> I'm afraid I can not share such files.
>
> Which flags are set in these files?
> How can I produce such a file?

Ping.

Until now, two types of files are supported:
Files with flags==0 and files with flags&0x02.
The specification requires that (flags&0x02)
implies flags==2 or possibly flags==6: "if bit
9 is 1, bits 11 and 12 must be 0"

After my patch, the following values are
accepted for flags: 0, 2, 4, 6

Which value is problematic?

Carl Eugen
Martin Vignali Feb. 8, 2018, 8:45 p.m. UTC | #7
>
> Until now, two types of files are supported:
> Files with flags==0 and files with flags&0x02.
> The specification requires that (flags&0x02)
> implies flags==2 or possibly flags==6: "if bit
> 9 is 1, bits 11 and 12 must be 0"
>
> After my patch, the following values are
> accepted for flags: 0, 2, 4, 6
>
>
Hello,

Don't have a strong opinion about this.
But like deep data is not supported, and multipart file output is not
correct
Maybe we can be more explicit about unsupported features, and ignore long
name bit

Like in patch in attach

Martin
Martin Vignali Feb. 17, 2018, 7:47 p.m. UTC | #8
2018-02-08 21:45 GMT+01:00 Martin Vignali <martin.vignali@gmail.com>:

>
>
>> Until now, two types of files are supported:
>> Files with flags==0 and files with flags&0x02.
>> The specification requires that (flags&0x02)
>> implies flags==2 or possibly flags==6: "if bit
>> 9 is 1, bits 11 and 12 must be 0"
>>
>> After my patch, the following values are
>> accepted for flags: 0, 2, 4, 6
>>
>>
> Hello,
>
> Don't have a strong opinion about this.
> But like deep data is not supported, and multipart file output is not
> correct
> Maybe we can be more explicit about unsupported features, and ignore long
> name bit
>
> Like in patch in attach
>
> Martin
>


Hello,

If no one is against, i will apply the patch in attach in few days (change
since previous patch (mention ticket in commit msg))

Martin
Martin Vignali Feb. 24, 2018, 8:47 p.m. UTC | #9
>
> Hello,
>
> If no one is against, i will apply the patch in attach in few days (change
> since previous patch (mention ticket in commit msg))
>
> Martin
>
>
Pushed
diff mbox

Patch

From 19f1896fc182f2bc8d1c715883ab40aaf17fc828 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Tue, 30 Jan 2018 10:24:08 +0100
Subject: [PATCH] lavc/exr: Ignore long names flag.

The decoder only reads known names, others are skipped.

Fixes ticket #6994.
---
 libavcodec/exr.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 454dc74..802618a 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1349,11 +1349,8 @@  static int decode_header(EXRContext *s, AVFrame *frame)
 
     flags = bytestream2_get_le24(&s->gb);
 
-    if (flags == 0x00)
-        s->is_tile = 0;
-    else if (flags & 0x02)
-        s->is_tile = 1;
-    else{
+    s->is_tile = !!(flags & 0x02);
+    if (flags & ~0x06) {
         avpriv_report_missing_feature(s->avctx, "flags %d", flags);
         return AVERROR_PATCHWELCOME;
     }
-- 
1.7.10.4