[FFmpeg-devel] [PATCH] mov demuxer crashes on certain .jp2 files

[Baptiste Coudurier]

Hi Jai,

Jai Menon wrote:
> Hi,
> 
> On Wed, Dec 17, 2008 at 12:20 AM, Baptiste Coudurier 
> <baptiste.coudurier at gmail.com> wrote:
>> Jai Menon wrote:
>>> Hi,
>>> 
>>> On Tue, Dec 16, 2008 at 2:20 PM, Baptiste Coudurier 
>>> <baptiste.coudurier at gmail.com> wrote:
>>>> Hi,
>>>> 
>>>> Jai Menon wrote:
>>>>> Hi,
>>>>> 
>>>>> The mov demuxer segfaults when parsing certain .jp2 files 
>>>>> found at :
>>>>> 
>>>>> http://samples.mplayerhq.hu/jpeg2000/j2kp4files_v1_2.zip/testfiles_jp2/*.jp2
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> Attached patch fixes this issue.
>>>>> 
>>>> Fixed in a slightly different way. We still need to figure out
>>>>  what is the best way to handle .jp2 and .mj2.
>>>> 
>>>> My first feeling is that the decoder will need to support 
>>>> 'atom' structure so .jp2 can be used with image2 format, maybe
>>>>  it does already, I don't know.
>>> jp2 is defined in itu recommendation t.800 annex I and it has the
>>>  same box/atom structure. there is a signature atom, ftyp, jp2h,
>>>  jp2c and other not so important atoms.
>>> 
>>>> .mj2 is ISO media with 'jp2h' atom in 'stsd', chunks of data 
>>>> are 'jp2c' atoms IIRC.
>>> from the motion jpeg2k (t.802) itu rec, section 4.3 :
>>> 
>>> 4.3 JP2 inheritance and compatibility ..... 2) The objects (boxes
>>>  or atoms) required by the JP2 specification shall also be 
>>> present. ....
>>> 
>>> so isn't it better that jp2 be handled by the mov demuxer itself?
>>>  this would basically mean handling the jp2h and jp2c atoms.
>> Why ? Please explain. I gave you an idea why not: "so .jp2 can be 
>> used with image2 format"
> 
> I'm not quite sure I understand. Are you implying that the img2 
> demuxer feed the file data to the jpeg2k decoder and the decoder 
> handle parsing of the atoms or ...

Yes that's what I mean.

ffmpeg -i test%d.jp2 will then work, like every image format basically.

>> In any way, mov demuxer will pass 'jp2c' atoms to decoder, so 
>> decoder has to support atom structure in some way, I think adding 
>> support for other boxes is trivial.
> 
> But I don't see any entry in the table for the jp2c atom. For the 
> purposes of decoding, iirc the jp2h atom isn't really important

Are you sure ? When I hooked jasper last time, I needed to pass data
contained in 'jp2h', if this is no needed then this is even simpler.

> and the mov_read_extradata function can be modified to just setup the
> streams and codec parameters and ignore most of data in it for the
> time being.

Well, this is really more hackish than handling atoms in the coder.

> The jp2c atom is just a monolithic jpeg2k codestream so the i'm not 
> sure if the decoder really needs to do anything complicated.

Parsing atoms isn't really complicated :>

> I was thinking along the lines of adding the jp2c tag to the parser
> table and a new function, but it seems to me that the mov demuxer
> requires that a moov atom be present. I think what is required is
> that the function, say mov_read_j2k_codestream read in the codestream
> and create a packet which can then be sent to the decoder. Is there 
> something wrong with this approach?

Well I'd say yes because you are hacking mov/mj2/isom demuxer when this
is not needed yet IMHO.

There are 2 situations:
.jp2: file containing 'jP', 'ftyp', 'jp2h', 'xml', 'jp2c'
.mj2: containing 'moov', 'jp2h' in 'stsd', and current demuxer will
output 'jp2c' atoms in AVPacket, basically 'mdat' atom contains 'jp2c'
atoms.

Implemeting a generic atom reading function in the decoder will avoid
hacking mov demuxer at all, and will enable ffmpeg -f image2 -i test%d.jp2

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no