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

[Jai Menon]

Hi,

On Wed, Dec 17, 2008 at 12:25 PM, Baptiste Coudurier
<baptiste.coudurier at gmail.com> wrote:
> 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.

Thats why I said "..for the time being" :-). Atleast the jpeg2k soc
decoder doesn't
care about extradata in the jp2h atom.
Then again, it doesn't really work all that well...

>> 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 :>

It isn't, what I meant was when the framework does exist, it seemed
like extra work :-)
iirc its hardly 10 lines of code in jasper.

>> 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

okay then, i'll just add it to the img2 code and do the rest in the j2k decoder.

-- 
Regards,

Jai