<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
On 05/12/2009 08:14 AM, Jean-Baptiste Kempf wrote:
<blockquote cite="mid:20090512151446.GA8415@videolan.org" type="cite">
<pre wrap="">On Tue, May 12, 2009 at 08:02:37AM -0700, John Stebbins wrote :
</pre>
<blockquote type="cite">
<pre wrap="">I'm adding vobsub support to handbrake and was using vlc as one means to
verify my work. I noticed that vlc wasn't using the palette contained
</pre>
</blockquote>
<pre wrap=""><!---->Great.
</pre>
<blockquote type="cite">
<pre wrap="">in the mkv track CodecPrivate data. When I looked deeper into it, I
discovered it was ignoring the idx data entirely. Here's a patch to
modules/demux/avformat/demux.c that checks that the container is
matroska, and that the subtitle type is vobsub. Then parses
AVCodecContext.extradata (where avformat stashes matroska track
CodecPrivate).
</pre>
</blockquote>
<pre wrap=""><!---->
I have to say that it is weird, since matroska demuxer should fire up
before avformat...
Do you have it (vlc --list) ?
</pre>
</blockquote>
vlc --list does not show mkv. I noticed when building that nothing in
modules/demux/mkv was compiled. I didn't look into why that was because
I thought it was intentional. If there is a build option that enables
it, then it needs to be fixed as well. If you happen to know the build
option, I can look into that.<br>
<blockquote cite="mid:20090512151446.GA8415@videolan.org" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ if (strncmp(p_sys->ic->iformat->name, "matroska", 8) == 0 &&
</pre>
</blockquote>
<pre wrap=""><!---->Coding style is not good, I would say...
</pre>
</blockquote>
What is it about the coding style you don't like? Looking at the
surrounding code, I would have to guess the spacing. You prefer this?<br>
if( strncmp( p_sys->ic->iformat->name, "matroska", 8 ) == 0 ...<br>
<blockquote cite="mid:20090512151446.GA8415@videolan.org" type="cite">
<pre wrap=""></pre>
<blockquote type="cite">
<pre wrap="">+ cc->codec_id == CODEC_ID_DVD_SUBTITLE)
+ {
+ char *pos;
+
+ pos = strstr((char*)cc->extradata, "size:");
</pre>
</blockquote>
<pre wrap=""><!---->Shouldn't it be (const char*)?
Coding style too.
</pre>
</blockquote>
strstr prototype is const char*, but it it perfectly valid to pass a
char* to a const char*. The const is only saying that strstr isn't
going to modify the content. It doesn't say that the content must be
unmodifiable.<br>
<br>
<blockquote cite="mid:20090512151446.GA8415@videolan.org" type="cite">
<pre wrap=""></pre>
<blockquote type="cite">
<pre wrap="">+ pos = strstr((char*)cc->extradata, "palette:");
</pre>
</blockquote>
<pre wrap=""><!---->Same.
</pre>
<blockquote type="cite">
<pre wrap="">+ int j;
+ for( j = 1; j < 17; j++ )
</pre>
</blockquote>
<pre wrap=""><!---->VLC is C99, so you can declare j in the for(, just FYI.
</pre>
</blockquote>
<blockquote cite="mid:20090512151446.GA8415@videolan.org" type="cite">
<pre wrap="">Anyway, it seems to work.
Best Regards,
</pre>
</blockquote>
Given the style issues, did you want me to fix it and resubmit?<br>
<br>
John<br>
<br>
</body>
</html>