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

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Sun, Jan 27, 2008 at 03:35:42PM +0100, Baptiste Coudurier wrote:
>> Hi,
>>
>> michael wrote:
>>> Author: michael
>>> Date: Sat Jan 26 23:57:53 2008
>>> New Revision: 11627
>>>
>>> Log:
>>> Set subtitle codec id correctly, i hope this doesnt break anything.
>>>
>>>
>>> Modified:
>>>    trunk/libavformat/mov.c
>>>
>>> Modified: trunk/libavformat/mov.c
>>> ==============================================================================
>>> --- trunk/libavformat/mov.c	(original)
>>> +++ trunk/libavformat/mov.c	Sat Jan 26 23:57:53 2008
>>> @@ -233,7 +233,6 @@ static int mov_read_hdlr(MOVContext *c, 
>>>          st->codec->codec_id = CODEC_ID_MP2;
>>>      else if(type == MKTAG('s', 'u', 'b', 'p')) {
>>>          st->codec->codec_type = CODEC_TYPE_SUBTITLE;
>>> -        st->codec->codec_id = CODEC_ID_DVD_SUBTITLE;
>>>      }
>>>      get_be32(pb); /* component  manufacture */
>>>      get_be32(pb); /* component flags */
>>> @@ -788,6 +787,8 @@ static int mov_read_stsd(MOVContext *c, 
>>>                  st->codec->bits_per_sample = bits_per_sample;
>>>                  sc->sample_size = (bits_per_sample >> 3) * st->codec->channels;
>>>              }
>>> +        } else if(st->codec->codec_type==CODEC_TYPE_SUBTITLE){
>>> +            st->codec->codec_id= id;
>>>          } else {
>>>              /* other codec type, just skip (rtp, mp4s, tmcd ...) */
>>>              url_fskip(pb, size - (url_ftell(pb) - start_pos));
>> This 'subp' media handler is the hackish way for nero to wrap dvd
>> subtitle, I don't think poluting objectype id is a good solution since I
>> removed the same standard one day ago.
>>
>> Does the play way with the old way to set codec id ? If so, I'd prefer
>> to revert the commit.
> 
> the old way did not work, thats why i changed it

Humm here without the commit:
ffmpeg -i unsupported-embedded-subs-2.mp4 -scodec copy test.vob

then mplayer displays subs correctly (with weird font though). I
remember that nero stores some extradata similar to what .ifo contains
in dvds, in esds.

So I guess for complete support, extradata should be extracted, I
remember a patch for that in ffmpeg-devel to support palette for subs in
mkv, and IIRC reimar have something like that in mplayer, which can be
ported to lavc, maybe Im wrong though.

> besides the object id is the codec identifer, the media type is the
> type identifer (video/audio/text/subtitle) hence its name "media type"
> also "subp" can hardly be explained as an abbreviation of vobsub or dvdsub
> its much closer to subtitle picture or subtitle page which would be
> generic media types independant of the used codec
> 
> and yes i fully agree with you that poluting object type id with it is
> bad, they should have registered an official one instead of using a private
> one, still if another files uses 224 as well the solution would be a
> if(codec_type == CODEC_TYPE_SUBTITLE) in the esds parsing code not hardcoding
> that all nero subtitles are vobsubs and then skiping he parsing of the actual
> codec identifer
> 

Well, if only forcing codec id like it was before this commit, is
sufficient, at least it still seems to me atm and it was when I first
commited it, using objectype id seems an overkill mess.

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