[FFmpeg-cvslog] r11621 - trunk/libavformat/mov.c

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Sun, Jan 27, 2008 at 03:38:24PM +0100, Baptiste Coudurier wrote:
>> Hi,
>>
>> michael wrote:
>>> Author: michael
>>> Date: Sat Jan 26 20:50:04 2008
>>> New Revision: 11621
>>>
>>> Log:
>>> Select non jpeg if there are multiple substreams.
>>>
>>>
>>> Modified:
>>>    trunk/libavformat/mov.c
>>>
>>> Modified: trunk/libavformat/mov.c
>>> ==============================================================================
>>> --- trunk/libavformat/mov.c	(original)
>>> +++ trunk/libavformat/mov.c	Sat Jan 26 20:50:04 2008
>>> @@ -600,8 +600,10 @@ static int mov_read_stsd(MOVContext *c, 
>>>          get_be16(pb); /* reserved */
>>>          get_be16(pb); /* index */
>>>  
>>> -        if (st->codec->codec_tag) {
>>> -            /* multiple fourcc, just skip for now */
>>> +        if (st->codec->codec_tag && st->codec->codec_tag != MKTAG('j', 'p', 'e', 'g')) {
>>> +            /* multiple fourcc, we skip jpeg, this isnt correct, we should export it as
>>> +               seperate AVStream but this needs a few changes in the mov demuxer, patch
>>> +               welcome */
>>>              url_fskip(pb, size - (url_ftell(pb) - start_pos));
>>>              continue;
>>>          }
>> This is hackish, 
> 
> yes i know
> 
> 
>> and Im against it. 
> 
>> You will have to add all image types
>> (png and so on). 
> 
> yes i will when i stumble across samples
> 
> 
>> The proper solution must be implemented, 
> 
> well a lot of things must be implemented, now if we had the manpower
> time, motivation, ...
> 
> 
>> and adding
>> hacks won't help.
> 
> that is the part i strongly disagree with
> it does help alot until (if ever) the correct solution is implemented

Here you have double standards, please remember the pcm 20 and 24 bits
in dvds patch a long time ago. Those files are still unplaybable, still
you didn't accept the patch.

> and its a very small isolated change, the correct solution requires large
> changes in the mov demuxer and in ffmpeg
> correct is to export the individual streams as seperate AVStreams, export
> their relation in some not yet existing API and then use the not yet in
> svn avfilter to merge them after deocoding,

Another way is to make possible the change of codec and notify it
through an AVPacket flag. I had a patch handling this correctly.

No need for avfilter.

> this also would solve the croping
> width/height in the mov header issue, that as well could be taken care of in
> a avfilter

Of course the cropping can be done in avfilter. Now exporting the
information is another problem, and is not related to exporting multiple
codecs in one stream.

> [...]
> i could bet that the correct solution wont be implemented in the next 3 years

How much would you bet ?

> i dont think its justified to have many mov files unplayable until then

It is justified for sake of avoiding ugly hacks like this.
Since the beginning those files aren't playbable with lavf,
and with any other opensource demuxers I know about.
One must jump in and implement it correctly, with this ugly hacks nobody
will jump in since 'it just works', and this rule applies to every hack
you discard for the sake of good implementation IMHO.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312