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

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Sun, Jan 27, 2008 at 07:21:02PM +0100, Baptiste Coudurier wrote:
>> 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
> 
> there are 3 related hunks
> 1. add the 224 object type
> 2. removial of "subp" -> CODEC_ID_DVD_SUBTITLE
> 3. add st->codec->codec_id= id;
> 
> without 3. "TEXT" subs have an unset codec_id and the ESDS atom is
> not parsed (and not parsing the ESDS is definitly not correct)
> -> 3. is needed (for TEXT subs)
> 
> if you now revert hunks 1.+2.
> CODEC_ID_DVD_SUBTITLE breaks, that is codec_id is no longer set
> 
> if you revert just 1. then it breaks as well (thers no 
> CODEC_ID_DVD_SUBTITLE anywhere anymore
> 
> if you revert just 2. that might work but it adds a useless line of
> code
> 
> if you revert 3. TEXT subs break, this also is true for all combinations
> with 3.
> 
> the case you apparently tested is reverting 1+2+3 or 2+3, these break
> TEXT subs not DVD subs
> 

My prefered solution is to avoid poluting object type id table with non
standard ones.

The support of nero hackish subs wrapping can be done without.
I did it in the first place. Now you just jumps in and commit hacks
without asking me first.

I'll cross my fingers and will hunt that PCM_S16_LE sample with object
type id 224.

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