Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Paul B Mahol]

On 1/6/16, wm4  wrote:
> On Wed, 6 Jan 2016 04:33:16 +0100
> Mats Peterson  wrote:
>
>> At least in this case, MPlayer seems to look at the codec tag rather
>> than the codec ID in order to determine the codec. Therefore, the
>> 'fourcc' variable needs to be set to 'SVQ3' as well, which is later
>> copied to st->codec->codec_tag in matroskadec.c.
>
> You should fix mplayer instead. FFmpeg API users are not supposed to
> use fourcc fields to identify codecs. The fourcc variable in
> matroskadec.c is copied to AVCodecContext.codec_tag, which is not
> necessarily a fourcc (depends on the CODEC_ID; most time it's unused),
> and the svq3 decoder in libavcodec doesn't read it.

Yes, fix broken MPlayer instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 11:06 AM, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:

[...]

The patch is ok, the commit message not:
This is not related to MPlayer but old
lavf producing broken files.
The codec_tag SMI does not exist, SVQ3
should be exported.

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



I think you should wait with your statements until you've tried things, 
Carl. The old (broken) "SMI" file output-ffmpeg-20140109-git-c0a33c4.mkv 
(Google for it), which is actually SVQ3, playsi without video in 
MPlayer. It complains at not finding a 'SMI ' codec.



Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Carl Eugen Hoyos]

Mats Peterson  ffmpeg.org> writes:

> You're welcome to fix this issue on the MPlayer side, 
> since it seems you've done that before.

If there were a fourcc (or codec_tag) "SMI " this 
would be trivial to fix in MPlayer.
But this codec_tag does not exist, FFmpeg used to 
write broken mkv files that contained only part 
of the stsd atom.
"Your" patch is correct, the codec_tag for mkv files 
containing SVQ3 should be "SVQ3".

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 11:26 AM, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


You're welcome to fix this issue on the MPlayer side,
since it seems you've done that before.


If there were a fourcc (or codec_tag) "SMI " this
would be trivial to fix in MPlayer.
But this codec_tag does not exist, FFmpeg used to
write broken mkv files that contained only part
of the stsd atom.
"Your" patch is correct, the codec_tag for mkv files
containing SVQ3 should be "SVQ3".

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



Yes, I know my patch is correct in that sense. One thing I don't 
understand is why MPlayer looks at the codec tag whatsoever in order to 
determine the video codec, when it should use the codec ID. But that's 
not my problem.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 11:26 AM, Carl Eugen Hoyos wrote:

But this codec_tag does not exist, FFmpeg used to
write broken mkv files that contained only part
of the stsd atom.


FFmpeg still writes broken mkv files, for the record. It doesn't include 
the palette, if any, in the stsd atom. But that's a later issue, one 
that I'm currently not concerned about since I use mkvmerge to remux.


Mats

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 12:22 PM, Mats Peterson wrote:

order to procuce a working Matroska file from a QuickTime file.


Produce, not procuce.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 10:41 AM, wm4 wrote:

On Wed, 6 Jan 2016 04:33:16 +0100
Mats Peterson  wrote:


At least in this case, MPlayer seems to look at the codec tag rather
than the codec ID in order to determine the codec. Therefore, the
'fourcc' variable needs to be set to 'SVQ3' as well, which is later
copied to st->codec->codec_tag in matroskadec.c.


You should fix mplayer instead. FFmpeg API users are not supposed to
use fourcc fields to identify codecs. The fourcc variable in
matroskadec.c is copied to AVCodecContext.codec_tag, which is not
necessarily a fourcc (depends on the CODEC_ID; most time it's unused),
and the svq3 decoder in libavcodec doesn't read it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



:) In any case, the fourcc should be SVQ3 and not SMI, regardless of 
what MPlayer is doing.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Carl Eugen Hoyos]

Mats Peterson  ffmpeg.org> writes:

[...]

The patch is ok, the commit message not:
This is not related to MPlayer but old 
lavf producing broken files.
The codec_tag SMI does not exist, SVQ3 
should be exported.

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 11:53 AM, Mats Peterson wrote:

FFmpeg still writes broken mkv files, for the record. It doesn't include
the palette, if any, in the stsd atom. But that's a later issue, one
that I'm currently not concerned about since I use mkvmerge to remux.



That's just one of the QuickTime to Matroska remuxing issues. But then 
again, I use mkvmerge, which is currently the safest alternative in 
order to procuce a working Matroska file from a QuickTime file.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[wm4]

On Wed, 6 Jan 2016 11:20:25 +0100
Mats Peterson  wrote:

> On 01/06/2016 11:16 AM, Mats Peterson wrote:
> > On 01/06/2016 11:12 AM, Mats Peterson wrote:  
> >> On 01/06/2016 11:06 AM, Carl Eugen Hoyos wrote:  
> >>> Mats Peterson  ffmpeg.org> writes:
> >>>
> >>> [...]
> >>>
> >>> The patch is ok, the commit message not:
> >>> This is not related to MPlayer but old
> >>> lavf producing broken files.
> >>> The codec_tag SMI does not exist, SVQ3
> >>> should be exported.
> >>>
> >>> Carl Eugen
> >>>
> >>> ___
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel@ffmpeg.org
> >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>  
> >>
> >> I think you should wait with your statements until you've tried things,
> >> Carl. The old (broken) "SMI" file output-ffmpeg-20140109-git-c0a33c4.mkv
> >> (Google for it), which is actually SVQ3, playsi without video in
> >> MPlayer. It complains at not finding a 'SMI ' codec.
> >>
> >>
> >> Mats
> >>  
> >
> > Or the fourcc 0x20494d53 rather.  (' ', 'I', 'M', 'S').
> >
> > Mats
> >  
> 
> You're welcome to fix this issue on the MPlayer side, since it seems 
> you've done that before.

Uh. You know, they accept patches, just like FFmpeg.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[wm4]

On Wed, 6 Jan 2016 04:33:16 +0100
Mats Peterson  wrote:

> At least in this case, MPlayer seems to look at the codec tag rather
> than the codec ID in order to determine the codec. Therefore, the
> 'fourcc' variable needs to be set to 'SVQ3' as well, which is later
> copied to st->codec->codec_tag in matroskadec.c.

You should fix mplayer instead. FFmpeg API users are not supposed to
use fourcc fields to identify codecs. The fourcc variable in
matroskadec.c is copied to AVCodecContext.codec_tag, which is not
necessarily a fourcc (depends on the CODEC_ID; most time it's unused),
and the svq3 decoder in libavcodec doesn't read it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 11:16 AM, Mats Peterson wrote:

On 01/06/2016 11:12 AM, Mats Peterson wrote:

On 01/06/2016 11:06 AM, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:

[...]

The patch is ok, the commit message not:
This is not related to MPlayer but old
lavf producing broken files.
The codec_tag SMI does not exist, SVQ3
should be exported.

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



I think you should wait with your statements until you've tried things,
Carl. The old (broken) "SMI" file output-ffmpeg-20140109-git-c0a33c4.mkv
(Google for it), which is actually SVQ3, playsi without video in
MPlayer. It complains at not finding a 'SMI ' codec.


Mats



Or the fourcc 0x20494d53 rather.  (' ', 'I', 'M', 'S').

Mats



You're welcome to fix this issue on the MPlayer side, since it seems 
you've done that before.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 11:32 AM, wm4 wrote:


Uh. You know, they accept patches, just like FFmpeg.

You don't say.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 10:53 AM, Paul B Mahol wrote:


Yes, fix broken MPlayer instead.

Me? :) Hardly.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Michael Niedermayer]

On Wed, Jan 06, 2016 at 10:26:08AM +, Carl Eugen Hoyos wrote:
> Mats Peterson  ffmpeg.org> writes:
> 
> > You're welcome to fix this issue on the MPlayer side, 
> > since it seems you've done that before.
> 
> If there were a fourcc (or codec_tag) "SMI " this 
> would be trivial to fix in MPlayer.
> But this codec_tag does not exist, FFmpeg used to 
> write broken mkv files that contained only part 
> of the stsd atom.

> "Your" patch is correct, the codec_tag for mkv files 
> containing SVQ3 should be "SVQ3".

fixed the commit message and applied

thx

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 02:45 PM, Michael Niedermayer wrote:

On Wed, Jan 06, 2016 at 10:26:08AM +, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


You're welcome to fix this issue on the MPlayer side,
since it seems you've done that before.


If there were a fourcc (or codec_tag) "SMI " this
would be trivial to fix in MPlayer.
But this codec_tag does not exist, FFmpeg used to
write broken mkv files that contained only part
of the stsd atom.



"Your" patch is correct, the codec_tag for mkv files
containing SVQ3 should be "SVQ3".


fixed the commit message and applied

thx

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire



___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



Thanks. Only the AV_CODEC_ID_NONE patches for matroskadec.c and mov.c 
left ;)


Mats

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

On 01/06/2016 11:53 AM, Mats Peterson wrote:

FFmpeg still writes broken mkv files, for the record. It doesn't include
the palette, if any, in the stsd atom. But that's a later issue, one
that I'm currently not concerned about since I use mkvmerge to remux.


To be perfectly clear, the private data in Matroska doesn't contain the 
whole stsd atom, but the *sample description*, which is *part* of the 
stsd atom.


Mats

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel