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

[Michael Niedermayer]

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
it creates race conditions with threads
breaks ALL remuxing (even to mov)
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?
should it free the codec and open another if the change happens?
what happens with seeking? what is in extradata?

also i fear that freeing the codec is wrong, the codec state must be
preserved from the previous frames

also i say it again, this is absolutely not limited to the first frame
any random frame in the middle can be a different codec, closing and opening
codecs will break their state. And expecting the user app to keep several
codecs per stream open and resolve all that with seeking and different
resolutions and colorspaces is hardly a solution ...

also theres a file (resurrection.mov) with RPZA and SVQ1 mixed and i emphasize
mixed
heres the qtdump of it to proof this:
       chunk 1 samples 1 id 1
       chunk 6 samples 1 id 2
       chunk 7 samples 3 id 2
       chunk 325 samples 1 id 2
       chunk 326 samples 1 id 1

> 
> No need for avfilter.

ohh well, why am i awnsering this
look if you have 2 codecs outputing different colorspaces and resolutions
you need to convert one otherwise you will have some problems with displaying
them i the same window with most video output drivers
this is also totally indepandant of AVPackets with flags or several AVStreams

also above file:
svq1 (PIX_FMT_YUV410P)
rpza (PIX_FMT_RGB555)


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

yes


> 
> > [...]
> > i could bet that the correct solution wont be implemented in the next 3 years
> 
> How much would you bet ?

10 euro if you have a bank account with IBAN/SWIFT i can transfer it to if i loose


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

mplayers demux_mov.c does play them AFAIK
you could say its luck, but its not
the first frame is often a jpeg, its less likely in the middle, so the first
stsd is jpeg, mplayer always uses the last, that simply us more reliable
than choosing the first blindly.


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

no, if it doesnt "just work" people use another player, or they will patch
the code with 1 or 2 lines to skip the jpeg like in the hack
noone will volunteerly implement it "cleanly" unless the simlpe hack doesnt
work but if it doesnt work it also doesnt hurt such implementation attempts

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20080127/2fafdc03/attachment.pgp>