[FFmpeg-devel] [PATCH] vobsub in mov support

[Baptiste Coudurier]

On 4/2/2009 4:40 AM, Reimar D?ffinger wrote:
> On Wed, Apr 01, 2009 at 11:39:46AM -0700, Baptiste Coudurier wrote:
>> Yes the "if (format..." is ok to apply.
> 
> Done.
> 
>>> And I do think there is no need to know the code to conclude that when for
>>> CODEC_TYPE_SUBTITLE all of codec_movaudio_tags, codec_movvideo_tags and
>>> codec_bmp_tags are checked for matches but ff_codec_movsubtitle_tags is not
>>> there either is a bug or it should be documented why.
>> Because CODEC_TYPE_SUBTITLE is set after matching the "fourcc", except
>> for this specific case (nero subs) because they use and special handler
>> type named "subp". Tracks are set to CODEC_TYPE_DATA by default and type
>> is updated after reading handler type. If handler type is fixed and
>> determined for 'tx3g' and 'text', then code in handler (hdlr) might be
>> updated to set type earlier.
> 
> Hm, I would have described it as "the code assumed there are only
> CODEC_TYPE_AUDIO, CODEC_TYPE_VIDEO and CODEC_TYPE_DATA and was not
> updated when CODEC_TYPE_SUBTITLE was added".

In fact code for the "subp" handler was added at the same time.
Code handling "tx3g", etc.. was added later IIRC.

> At least that's what the comment on this line looks like:
>> } else if (st->codec->codec_type != CODEC_TYPE_AUDIO && /* do not overwrite codec type */
> 
> The code basically looks like this now:
> search audio id
> if (!is VIDEO && id found)
>   is AUDIO
> else if (!is AUDIO) {
>   search video id
>   if (id found) is VIDEO
>   else if (is DATA) {
>     search sub id
>     if (id found) is SUB
>   }
> }
> 
> While IMHO what was _meant_ is
> if (is AUDIO or is DATA) {
>   search audio id
>   if (id found) is AUDIO
> }

This does not work because "raw " is used both in video and audio.
In this case handler has precedence.

> or am I missing something?

I think your idea is correct, however this code has been here for quite
some time now, and it does not seem necessary to change it until we
encounter a real problem.

I'll also clean it a bit when we have the handler type available in all
cases, this will be done after "stsd" extraction is done.

I'm still waiting for inputs on the patch:
"[RFC] using ByteIOContext with buffer without reading function"

Michael said <= should be ok. I think it should applied.

> I just meant to say, if I am missing something why my second version
> would not be correct that should be explained by a comment.
> If it is correct, are you interested in a patch to change it like that?
> Note that I think the current code will do funny stuff for a (probably
> invalid file) where e.g. handler == "vide" and format == "QDM2", you will
> end up with a video stream with CODEC_ID_QDM2, which I think is about
> the silliest way to handle it :-P ...

Well, the question is how a video stream with the same "fourc" as QDM2
audio should be handled ? According to the usage of the "raw " fourcc,
it appears that the handler as precedence.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org