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

[Michael Niedermayer]

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


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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/64ff9313/attachment.pgp>