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

Baptiste Coudurier baptiste.coudurier
Mon Jan 28 01:30:07 CET 2008


Michael Niedermayer wrote:
> On Sun, Jan 27, 2008 at 11:19:11PM +0100, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> On Sun, Jan 27, 2008 at 07:29:35PM +0100, Baptiste Coudurier wrote:
>>>> 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.
>>> iam not mans, you are mixing people up here
>>>
>>> iam not mpeg-ps maintainer and wouldnt strongly mind a hack in mpeg-ps
>>> to work around lpcm problems, though the correct place is a bitstream
>>> filter
>>> and doing that in a bitstream filter would be trivial its just noone
>>> copy and pasted the code in one
>>> you hardly can compare that to designing a new API, rewriting 1/3 of the
>>> mov demuxer, ffmpeg.c and getting libavfilter into shape
>>>
>>>
>>>>> 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.
>>> sorry, you cannot change the codec in an AVStream this violates the API
>> API can be extended, like it was many times already.
> 
> the API isnt the problem
> its that you need all this information during writing the header
> you cant just update it randomly with AVPackets
> 
> 
>>> it creates race conditions with threads
>> No.
> 
> If you set a RESET_CODEC flag in an AVPacket and update
> codec_id width/height/extradata in AVCodecContext from the demuxer then the
> decoder which can run in a seperate thread (see ffplay) will have these
> values change below its fingers while still having older AVPackets to
> decode

Hell, Im not suggesting changing any value in AVCodecContext, Im
suggesting having more than one AVCodecContext.

> so, there is a race condition
> 
> 
>>> breaks ALL remuxing (even to mov)
>> No it does not break anything, if it's done well.
>> Now don't even try to remux this stream into any other container not
>> supporting this feature, this would be pointless, and real stream copy
>> in mov case is to remux the whole stream in one track, not splitting them.
> 
> its not pointless to remux one of the streams (for example the svq1 one
> droping the rpza)
> with multiple AVStreams thats just works

And also with substream ids and multiple AVCodecContext.

>>> and its not the codec its everything that can change including resolution
>>>
>>> also, lets for the moment ignore the issues above
>>> how is the user app supposed to handle it?
>> We can discuss about that.
>>
> 
>>> should it free the codec and open another if the change happens?
>> Why freeing codec ? You free the codec at the end of the demuxing,
>> like it's being done atm.
> 
> you want to feed the svq1 in the rpza decoder? if no you either 
> A. close it (will break following P frames)
> B. keep several codecs open per stream

Keep several, yes, like in the multiple AVStream case, I guess you are
not suggesting to close/open it for each frame if you want to display
both AVStream interleaved (which is the correct way).

> [...]
>>> also i fear that freeing the codec is wrong, the codec state must be
>>> preserved from the previous frames
>> This is just pure guess, and I do guess it's not the case.
> 
> So you are designing APIs based on guessing? Why dont you try it?
> Its a matter of hacking an existing mov so that a frame in the
> middle of lets say a svq1 stream is a jpeg, if the following P frame
> isnt black/green then QT preserves the codec state.

More like wether it is allowed or not in quicktime specs, but still you
can keep all AVCodecContext open.

> [...]
>> The whole concept in mov is specific to mov and you keep wanting formats
>> to adapt to your view of libavformat API. I don't.
> 
> The problem is nothing these days is specific to any container
> Just think of mpeg4, it can have multiple objects.
> Its a more general case then mov switch per frame, mpeg4 allows
> several streams and combines them, this is similar to mov and should
> use the same API. Sadly your design would not be capable to handle it
> 
> and yes multiple object mpeg4 and scalability was what i had in mind
> when suggested the multple AVStream solution, if these were implemented
> the mov substreams would naturally fall into it
> 

Well I don't know enough about how is svc implemented in mp4 so I might
say something wrong, but here we are dealing with consecutive frames in
only one stream having a different codec, svc would add some information
to an already continuous avc codec, and I don't know if svc file format
use one avc + one svc stsd entry.

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




More information about the ffmpeg-cvslog mailing list