[FFmpeg-devel] [PATCH] WebM mux/demux
James Zern
jzern
Fri May 21 01:46:47 CEST 2010
On Thu, May 20, 2010 at 19:23, Aurelien Jacobs <aurel at gnuage.org> wrote:
> On Thu, May 20, 2010 at 04:27:02AM -0400, David Conrad wrote:
>> On May 19, 2010, at 6:14 PM, Ronald S. Bultje wrote:
>>
>> > Hi,
>> >
>> > On Wed, May 19, 2010 at 5:22 PM, David Conrad <lessen42 at gmail.com> wrote:
>> >> (for certain definitions of fixed...)
>> > [..]
>> >> + ? ? ? ?int probelen = strlen(matroska_doctypes[i]);
>> >> + ? ? ? ?for (n = 4+size; n <= 4+size+total-(probelen-1); n++)
>> >> + ? ? ? ? ? ?if (!memcmp(p->buf+n, matroska_doctypes[i], probelen-1))
>> >
>> > Why -1? The original code has -1 because sizeof(str) includes the
>> > terminating zero. strlen() doesn't count that, so the -1 becomes
>> > unnecessary.
>>
>> Zombie coding.
>>
>> > Otherwise yes, but Baptiste is maintainer.
>>
>> You meant Aurel?
>>
>
>> commit d21f6fd54a9b624321ad208959c5de38f138b324
>> Author: David Conrad <lessen42 at gmail.com>
>> Date: ? Wed May 19 13:10:33 2010 -0400
>>
>> ? ? matroskadec: Add webm doctype
>>
>> ? ? Based on a Google patch
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 1bde1af..a287b36 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -209,6 +209,7 @@ typedef struct {
>>
>> ?typedef struct {
>> ? ? ?AVFormatContext *ctx;
>> + ? ?const char *doctype;
>
> Why ? It don't look useful... Could be a local var.
>
>> [...]
>> ? ? ? ? ?av_log(matroska->ctx, AV_LOG_ERROR,
>> ? ? ? ? ? ? ? ? "EBML header using unsupported features\n"
>> - ? ? ? ? ? ? ? "(EBML version %"PRIu64", doctype %s, doc version %"PRIu64")\n",
>> - ? ? ? ? ? ? ? ebml.version, ebml.doctype, ebml.doctype_version);
>> + ? ? ? ? ? ? ? "(EBML version %"PRIu64", doc version %"PRIu64")\n",
>> + ? ? ? ? ? ? ? ebml.version, ebml.doctype_version);
>
> I'm not sure it's a good idea to remove doctype from this message.
> Does it do any harm ?
>
>> + ? ?for (i = 0; i < FF_ARRAY_ELEMS(matroska_doctypes); i++)
>> + ? ? ? ?if (!strcmp(ebml.doctype, matroska_doctypes[i])) {
>> + ? ? ? ? ? ?matroska->doctype = matroska_doctypes[i];
>> + ? ? ? ? ? ?break;
>> + ? ? ? ?}
>> + ? ?if (!matroska->doctype) {
>
> Here you could just do :
> ?if (i >= FF_ARRAY_ELEMS(matroska_doctypes))
> and totally drop matroska->doctype.
>
> Except this, patch looks OK.
>
Here is a roll-up of what was discussed on the demux side, including a
free for the ebml on the patch welcome returns.
In addition this also stores the doctype similar to how the mov
demuxer stores the major brand/minor version.
-------------- next part --------------
Index: Changelog
===================================================================
--- Changelog (revision 23203)
+++ Changelog (working copy)
@@ -71,6 +71,7 @@ version <next>:
- spectral extension support in the E-AC-3 decoder
- unsharp video filter
- RTP hinting in the mov/3gp/mp4 muxer
+- WebM support in Matroska demuxer
Index: doc/general.texi
===================================================================
--- doc/general.texi (revision 23203)
+++ doc/general.texi (working copy)
@@ -234,6 +234,7 @@ library:
@item VC-1 test bitstream @tab X @tab X
@item WAV @tab X @tab X
@item WavPack @tab @tab X
+ at item WebM @tab @tab X
@item Wing Commander III movie @tab @tab X
@tab Multimedia format used in Origin's Wing Commander III computer game.
@item Westwood Studios audio @tab @tab X
Index: libavformat/matroskadec.c
===================================================================
--- libavformat/matroskadec.c (revision 23203)
+++ libavformat/matroskadec.c (working copy)
@@ -505,6 +505,8 @@ static EbmlSyntax matroska_clusters[] =
{ 0 }
};
+static const char *matroska_doctypes[] = { "matroska", "webm" };
+
/*
* Return: Whether we reached the end of a level in the hierarchy or not.
*/
@@ -825,8 +827,7 @@ static void ebml_free(EbmlSyntax *syntax
static int matroska_probe(AVProbeData *p)
{
uint64_t total = 0;
- int len_mask = 0x80, size = 1, n = 1;
- static const char probe_data[] = "matroska";
+ int len_mask = 0x80, size = 1, n = 1, i;
/* EBML header? */
if (AV_RB32(p->buf) != EBML_ID_HEADER)
@@ -848,13 +849,16 @@ static int matroska_probe(AVProbeData *p
if (p->buf_size < 4 + size + total)
return 0;
- /* The header must contain the document type 'matroska'. For now,
+ /* The header should contain a known document type. For now,
* we don't parse the whole header but simply check for the
* availability of that array of characters inside the header.
* Not fully fool-proof, but good enough. */
- for (n = 4+size; n <= 4+size+total-(sizeof(probe_data)-1); n++)
- if (!memcmp(p->buf+n, probe_data, sizeof(probe_data)-1))
- return AVPROBE_SCORE_MAX;
+ for (i = 0; i < FF_ARRAY_ELEMS(matroska_doctypes); i++) {
+ int probelen = strlen(matroska_doctypes[i]);
+ for (n = 4+size; n <= 4+size+total-probelen; n++)
+ if (!memcmp(p->buf+n, matroska_doctypes[i], probelen))
+ return AVPROBE_SCORE_MAX;
+ }
return 0;
}
@@ -1141,12 +1145,23 @@ static int matroska_read_header(AVFormat
/* First read the EBML header. */
if (ebml_parse(matroska, ebml_syntax, &ebml)
|| ebml.version > EBML_VERSION || ebml.max_size > sizeof(uint64_t)
- || ebml.id_length > sizeof(uint32_t) || strcmp(ebml.doctype, "matroska")
- || ebml.doctype_version > 2) {
+ || ebml.id_length > sizeof(uint32_t) || ebml.doctype_version > 2) {
av_log(matroska->ctx, AV_LOG_ERROR,
"EBML header using unsupported features\n"
"(EBML version %"PRIu64", doctype %s, doc version %"PRIu64")\n",
ebml.version, ebml.doctype, ebml.doctype_version);
+ ebml_free(ebml_syntax, &ebml);
+ return AVERROR_PATCHWELCOME;
+ }
+ for (i = 0; i < FF_ARRAY_ELEMS(matroska_doctypes); i++)
+ if (!strcmp(ebml.doctype, matroska_doctypes[i])) {
+ av_log(s, AV_LOG_DEBUG, "EBML doctype '%s'\n", ebml.doctype);
+ av_metadata_set2(&s->metadata, "doctype", ebml.doctype, 0);
+ break;
+ }
+ if (i >= FF_ARRAY_ELEMS(matroska_doctypes)) {
+ av_log(s, AV_LOG_ERROR, "Unknown EBML doctype '%s'\n", ebml.doctype);
+ ebml_free(ebml_syntax, &ebml);
return AVERROR_PATCHWELCOME;
}
ebml_free(ebml_syntax, &ebml);
Index: libavformat/avformat.h
===================================================================
--- libavformat/avformat.h (revision 23203)
+++ libavformat/avformat.h (working copy)
@@ -22,7 +22,7 @@
#define AVFORMAT_AVFORMAT_H
#define LIBAVFORMAT_VERSION_MAJOR 52
-#define LIBAVFORMAT_VERSION_MINOR 63
+#define LIBAVFORMAT_VERSION_MINOR 64
#define LIBAVFORMAT_VERSION_MICRO 0
#define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
More information about the ffmpeg-devel
mailing list